diff --git a/keepercommander/api.py b/keepercommander/api.py index ad83ace7a..77c76717d 100644 --- a/keepercommander/api.py +++ b/keepercommander/api.py @@ -550,6 +550,42 @@ def search_shared_folders(params, searchstring, use_regex=False): return search_results +def search_nested_share_folders(params, searchstring, use_regex=False): + """Search Nested Share Folders (v3 folder tree). + + Args: + params: KeeperParams + searchstring: Search string (tokens or regex depending on use_regex) + use_regex: If True, treat as regex. If False (default), token-based search. + If searchstring is empty, returns all Nested Share Folders. + + Returns: + List of (folder_uid, name) tuples. + """ + nsf_folders = getattr(params, 'nested_share_folders', {}) or {} + + if not searchstring: + match_func = lambda target: True + elif use_regex: + p = re.compile(searchstring.lower()) + match_func = lambda target: p.search(target) + else: + tokens = [t.lower() for t in searchstring.split() if t.strip()] + if not tokens: + match_func = lambda target: True + else: + match_func = lambda target: all(token in target for token in tokens) + + search_results = [] + for folder_uid, folder in nsf_folders.items(): + name = folder.get('name', '') + target = (folder_uid + ' ' + name).lower() + if match_func(target): + search_results.append((folder_uid, name)) + + return search_results + + def search_teams(params, searchstring, use_regex=False): """Search teams. diff --git a/keepercommander/commands/discoveryrotation.py b/keepercommander/commands/discoveryrotation.py index c8c3bd13b..5bc862e60 100644 --- a/keepercommander/commands/discoveryrotation.py +++ b/keepercommander/commands/discoveryrotation.py @@ -35,6 +35,7 @@ get_vault_record_title_type, find_pam_records_by_search, resolve_pam_config_folder_info, pam_folder_json_payload, place_record_in_folder, create_pam_configuration_in_folder, update_pam_record, records_in_folder, + reload_pam_record_if_nsf_updated, ) from .pam.config_helper import configuration_controller_get, \ pam_configurations_get_all, pam_configuration_remove, \ @@ -75,13 +76,11 @@ from .discover.rule_list import PAMGatewayActionDiscoverRuleListCommand from .discover.rule_remove import PAMGatewayActionDiscoverRuleRemoveCommand from .discover.rule_update import PAMGatewayActionDiscoverRuleUpdateCommand -from .pam_debug.acl import PAMDebugACLCommand from .pam_debug.dump import PAMDebugDumpCommand from .pam_debug.gateway import PAMDebugGatewayCommand from .pam_debug.graph import PAMDebugGraphCommand from .pam_debug.info import PAMDebugInfoCommand from .pam_debug.krouter import PAMDebugKRouterCommand -from .pam_debug.link import PAMDebugLinkCommand from .pam_debug.rotation_setting import PAMDebugRotationSettingsCommand from .pam_debug.vertex import PAMDebugVertexCommand from .pam.cnapp_commands import PAMCnappCommand @@ -463,11 +462,6 @@ def __init__(self): self.register_command('gateway', PAMDebugGatewayCommand(), 'Debug a gateway', 'g') self.register_command('krouter', PAMDebugKRouterCommand(), 'Show connected krouter version', 'k') self.register_command('graph', PAMDebugGraphCommand(), 'Render graphs', 'r') - - # Disable for now. Needs more work. - # self.register_command('verify', PAMDebugVerifyCommand(), 'Verify graphs') - self.register_command('acl', PAMDebugACLCommand(), 'Control ACL of PAM Users', 'c') - self.register_command('link', PAMDebugLinkCommand(), 'Link resource to configuration', 'l') self.register_command('rs-reset', PAMDebugRotationSettingsCommand(), 'Create/reset rotation settings', 'rs') self.register_command('vertex', PAMDebugVertexCommand(), @@ -2435,7 +2429,7 @@ def print_root_rotation_setting(params, is_verbose=False, format_type='table'): for c in configurations: # type: vault.TypedRecord if c.record_type in ('pamAwsConfiguration', 'pamAzureConfiguration', 'pamGcpConfiguration', 'pamDomainConfiguration', 'pamNetworkConfiguration', 'pamOciConfiguration', - 'pamGitHubConfiguration'): + 'pamGitHubConfiguration', 'pamHashiCorpConfiguration'): facade.record = c folder_info = resolve_pam_config_folder_info( params, facade, c.record_uid) @@ -2495,7 +2489,7 @@ def print_root_rotation_setting(params, is_verbose=False, format_type='table'): common_parser = argparse.ArgumentParser(add_help=False) common_parser.add_argument('--environment', '-env', dest='config_type', action='store', - choices=['local', 'aws', 'azure', 'gcp', 'domain', 'oci', 'github'], help='PAM Configuration Type') + choices=['local', 'aws', 'azure', 'gcp', 'domain', 'oci', 'github', 'hashicorp'], help='PAM Configuration Type') 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', @@ -2561,12 +2555,23 @@ def print_root_rotation_setting(params, is_verbose=False, format_type='table'): github_group.add_argument('--github-base-url', dest='github_base_url', action='store', help='GitHub Base URL') +hashicorp_group = common_parser.add_argument_group('hashicorp', 'HashiCorp Vault configuration') +hashicorp_group.add_argument('--hashicorp-id', dest='hashicorp_id', action='store', help='HashiCorp Id') +hashicorp_group.add_argument('--vault-base-url', dest='vault_base_url', action='store', + help='Vault Base URL (e.g., https://vault.company.com:8200)') +hashicorp_group.add_argument('--vault-token', dest='vault_token', action='store', + help='Vault Token (optional; syncIdentity takes precedence)') +hashicorp_group.add_argument('--vault-namespace', dest='vault_namespace', action='store', + help='Vault Namespace (optional; leave blank for Community Edition)') +hashicorp_group.add_argument('--vault-mount-path', dest='vault_mount_path', action='store', + help='Vault KV Mount Path (optional; defaults to "secret")') + class PamConfigurationEditMixin(RecordEditMixin): pam_record_types = None PAM_CONFIG_RECORD_TYPES = frozenset({ 'pamAwsConfiguration', 'pamAzureConfiguration', 'pamGcpConfiguration', 'pamDomainConfiguration', 'pamNetworkConfiguration', 'pamOciConfiguration', - 'pamGitHubConfiguration', + 'pamGitHubConfiguration', 'pamHashiCorpConfiguration', }) PAM_RESOURCE_RECORD_TYPES = frozenset({ 'pamDatabase', 'pamDirectory', 'pamMachine', 'pamRemoteBrowser', @@ -2865,6 +2870,22 @@ def parse_properties(self, params, record, **kwargs): # type: (KeeperParams, va oci_region = kwargs.get('oci_region') if oci_region: extra_properties.append(f'text.regionOci={oci_region}') + elif record.record_type == 'pamHashiCorpConfiguration': + hashicorp_id = kwargs.get('hashicorp_id') + if hashicorp_id: + extra_properties.append(f'text.pamHashiCorpId={hashicorp_id}') + vault_base_url = kwargs.get('vault_base_url') + if vault_base_url: + extra_properties.append(f'text.pamHashiCorpVaultBaseUrl={vault_base_url}') + vault_token = kwargs.get('vault_token') + if vault_token: + extra_properties.append(f'secret.pamHashiCorpVaultToken={vault_token}') + vault_namespace = kwargs.get('vault_namespace') + if vault_namespace: + extra_properties.append(f'text.pamHashiCorpVaultNamespace={vault_namespace}') + vault_mount_path = kwargs.get('vault_mount_path') + if vault_mount_path: + extra_properties.append(f'text.pamHashiCorpVaultMountPath={vault_mount_path}') if extra_properties: self.assign_typed_fields(record, [RecordEditMixin.parse_field(x) for x in extra_properties]) @@ -2942,9 +2963,11 @@ def execute(self, params, **kwargs): record_type = 'pamDomainConfiguration' elif config_type == 'oci': record_type = 'pamOciConfiguration' + elif config_type == 'hashicorp': + record_type = 'pamHashiCorpConfiguration' else: raise CommandError('pam-config-new', f'--environment {config_type} is not supported' - ' - supported options: local, aws, azure, gcp, domain, oci, github') + ' - supported options: local, aws, azure, gcp, domain, oci, github, hashicorp') title = kwargs.get('title') if not title: @@ -3128,7 +3151,8 @@ def execute(self, params, **kwargs): self.parse_properties(params, configuration, config_edit=True, **kwargs) self.verify_required(configuration, command='pam-config-edit') - update_pam_record(params, configuration, command='pam-config-edit') + was_nsf = update_pam_record(params, configuration, command='pam-config-edit') + configuration = reload_pam_record_if_nsf_updated(params, configuration, configuration.record_uid, was_nsf) admin_cred_ref = '' value = field.get_default_value(dict) diff --git a/keepercommander/commands/folder.py b/keepercommander/commands/folder.py index 7da2d0da8..ade0054c6 100644 --- a/keepercommander/commands/folder.py +++ b/keepercommander/commands/folder.py @@ -231,7 +231,7 @@ def _load_record_for_ls(params, uid): rec = vault.TypedRecord(version=3) rec.record_uid = uid rec.title = dj.get('title', uid) - rec.record_type = dj.get('type', '') + rec.type_name = dj.get('type', '') rec.load_record_data(dj, None) return rec return None diff --git a/keepercommander/commands/nested_share_folder/folder_commands.py b/keepercommander/commands/nested_share_folder/folder_commands.py index 28f3a677f..2f1a95922 100644 --- a/keepercommander/commands/nested_share_folder/folder_commands.py +++ b/keepercommander/commands/nested_share_folder/folder_commands.py @@ -530,7 +530,7 @@ def _apply(cls, params, action, folder_uid, recipient, role, expiration, logging.info("%s share '%s' %s", kind, recipient, verb) else: logging.warning("%s share '%s' failed", kind, recipient) - except ValueError as e: + except _nsf.ShareInviteSentError as e: logging.warning("nsf-share-folder: %s", e) except Exception as e: raise CommandError('nsf-share-folder', str(e)) diff --git a/keepercommander/commands/nested_share_folder/sharing_commands.py b/keepercommander/commands/nested_share_folder/sharing_commands.py index 8872ba78f..5e66a7545 100644 --- a/keepercommander/commands/nested_share_folder/sharing_commands.py +++ b/keepercommander/commands/nested_share_folder/sharing_commands.py @@ -106,9 +106,13 @@ def execute(self, params, **kwargs): raise_if_record_share_target_is_owner( params, record_uid, email, 'nsf-share-record', is_ownership_transfer=(action == 'owner')) - result, effective_action = self._dispatch( - params, action, record_uid, email, access_role_type, expiration, - rotate_on_expiration) + try: + result, effective_action = self._dispatch( + params, action, record_uid, email, access_role_type, expiration, + rotate_on_expiration) + except _nsf.ShareInviteSentError as e: + logging.warning('nsf-share-record: %s', e) + continue self._log_results(result, effective_action, email) # Strategy dispatch — returns (result, effective_action) diff --git a/keepercommander/commands/pam/vault_target.py b/keepercommander/commands/pam/vault_target.py index 91cd019ad..adfc63342 100644 --- a/keepercommander/commands/pam/vault_target.py +++ b/keepercommander/commands/pam/vault_target.py @@ -566,8 +566,31 @@ def is_pam_nsf_record(params, record_uid): return False -def update_pam_record(params, record, command='pam', force_nsf=False): +def reload_pam_record_if_nsf_updated(params, record, record_uid, was_nsf_updated): + """Reload a PAM record from cache if it was updated via NSF. + + After NSF updates, the in-memory record object becomes stale. This helper + reloads it from the refreshed cache. For classic updates, returns the record + unchanged since classic updates use deferred sync (no immediate cache refresh). + + Raises CommandError if NSF update occurred but reload fails, rather than + proceeding with known-stale data (which reintroduces the field-reversion bug). + """ + if was_nsf_updated: + from ..pam_import.record_loader import load_pam_record + reloaded = load_pam_record(params, record_uid) + if not reloaded: + raise CommandError('pam', f'Failed to reload NSF-updated record {record_uid} from cache after sync. ' + 'Record data may be stale; aborting edit to prevent field reversion.') + return reloaded + return record + + +def update_pam_record(params, record, command='pam', force_nsf=False) -> bool: """Update a PAM record via NSF v3 API or classic record_management. + + Returns True if the record was updated via NSF and the in-memory object is now stale + and must be reloaded from cache. Returns False for classic updates using deferred sync. """ from ..nested_share_folder.helpers import normalize_nsf_user_message from ...nested_share_folder.record_api import update_record_v3 @@ -587,11 +610,13 @@ def update_pam_record(params, record, command='pam', force_nsf=False): 'Failed to update record in Nested Share Folder') from ..pam_import.nsf_helpers import sync_down_preserving_nsf_keys sync_down_preserving_nsf_keys(params) + return True else: record_management.update_record(params, record) # Defer vault refresh so a second classic edit in the same session does # not send a stale record_cache revision (no immediate sync_down here). params.sync_data = True + return False def execute_record_add_in_folder(params, args, folder_uid, command='pam'): diff --git a/keepercommander/commands/pam_debug/acl.py b/keepercommander/commands/pam_debug/acl.py deleted file mode 100644 index 3b1430646..000000000 --- a/keepercommander/commands/pam_debug/acl.py +++ /dev/null @@ -1,156 +0,0 @@ -from __future__ import annotations -import argparse -import logging -from ..discover import (PAMGatewayActionDiscoverCommandBase, GatewayContext, PAM_USER, MultiConfigurationException, - multi_conf_msg) -from ...display import bcolors -from . import load_pam_record -from ...discovery_common.record_link import RecordLink -from ...discovery_common.types import UserAcl -from typing import TYPE_CHECKING - -if TYPE_CHECKING: - from ...vault import TypedRecord - from ...params import KeeperParams - - -class PAMDebugACLCommand(PAMGatewayActionDiscoverCommandBase): - parser = argparse.ArgumentParser(prog='pam action debug acl') - - # The record to base everything on. - parser.add_argument('--gateway', '-g', required=True, dest='gateway', action='store', - help='Gateway name or UID.') - parser.add_argument('--configuration-uid', "-c", required=False, dest='configuration_uid', - action='store', help='PAM configuration UID, if gateway has multiple.') - - parser.add_argument('--user-uid', '-u', required=True, dest='user_uid', action='store', - help='User UID.') - parser.add_argument('--parent-uid', '-r', required=True, dest='parent_uid', action='store', - help='Resource or Configuration UID.') - parser.add_argument('--debug-gs-level', required=False, dest='debug_level', action='store', - help='GraphSync debug level. Default is 0', type=int, default=0) - - def get_parser(self): - return PAMDebugACLCommand.parser - - def execute(self, params: KeeperParams, **kwargs): - - gateway = kwargs.get("gateway") - user_uid = kwargs.get("user_uid") - parent_uid = kwargs.get("parent_uid") - debug_level = int(kwargs.get("debug_level", 0)) - - print("") - - configuration_uid = kwargs.get('configuration_uid') - try: - gateway_context = GatewayContext.from_gateway(params=params, - gateway=gateway, - configuration_uid=configuration_uid) - if gateway_context is None: - print(f"{bcolors.FAIL}Could not find the gateway configuration for {gateway}.{bcolors.ENDC}") - return - except MultiConfigurationException as err: - multi_conf_msg(gateway, err) - return - - record_link = RecordLink(record=gateway_context.configuration, - params=params, - logger=logging, - debug_level=debug_level, use_per_graph_endpoints=False) - - user_record = load_pam_record(params, user_uid) # type: TypedRecord | None - if user_record is None: - print(f"{bcolors.FAIL}The user record does not exists.{bcolors.ENDC}") - return - - print(f"{bcolors.BOLD}The user record is {user_record.title}{bcolors.ENDC}") - - if user_record.record_type != PAM_USER: - print(f"{bcolors.FAIL}The user record is not a PAM User record.{bcolors.ENDC}") - return - - parent_record = load_pam_record(params, parent_uid) # type: TypedRecord | None - if parent_record is None: - print(f"{bcolors.FAIL}The parent record does not exists.{bcolors.ENDC}") - return - - print(f"{bcolors.BOLD}The parent record is {parent_record.title}{bcolors.ENDC}") - - if parent_record.record_type.startswith("pam") is False: - print(f"{bcolors.FAIL}The parent record is not a PAM record.{bcolors.ENDC}") - return - - if parent_record.record_type == PAM_USER: - print(f"{bcolors.FAIL}The parent record cannot be a PAM User record.{bcolors.ENDC}") - return - - parent_is_config = parent_record.record_type.endswith("Configuration") - - # Get the ACL between the user and the parent. - # It might not exist. - acl_exists = True - acl = record_link.get_acl(user_uid, parent_uid) - if acl is None: - print("No existing ACL, creating an ACL.") - acl = UserAcl() - acl_exists = False - - # Make sure the ACL for cloud user is set. - if parent_is_config is True: - print("Is an IAM user.") - acl.is_iam_user = True - - rl_parent_vertex = record_link.dag.get_vertex(parent_uid) - if rl_parent_vertex is None: - print("Parent record linking vertex did not exists, creating one.") - rl_parent_vertex = record_link.dag.add_vertex(parent_uid) - - rl_user_vertex = record_link.dag.get_vertex(user_uid) - if rl_user_vertex is None: - print("User record linking vertex did not exists, creating one.") - rl_user_vertex = record_link.dag.add_vertex(user_uid) - - has_admin_uid = record_link.get_admin_record_uid(parent_uid) - if has_admin_uid is not None: - print("Parent record already has an admin.") - else: - print("Parent record does not have an admin.") - - belongs_to_vertex = record_link.acl_has_belong_to_record_uid(user_uid) - if belongs_to_vertex is None: - print("User record does not belong to any resource, or provider.") - else: - if not belongs_to_vertex.active: - print("User record belongs to an inactive parent.") - else: - print("User record belongs to another record.") - - print("") - - while True: - res = input(f"Does this user belong to {parent_record.title} Y/N >").lower() - if res == "y": - acl.belongs_to = True - break - elif res == "n": - acl.belongs_to = False - break - - if has_admin_uid is None: - while True: - res = input(f"Is this user the admin of {parent_record.title} Y/N >").lower() - if res == "y": - acl.is_admin = True - break - elif res == "n": - acl.is_admin = False - break - - try: - record_link.belongs_to(user_uid, parent_uid, acl=acl) - record_link.save() - print(f"{bcolors.OKGREEN}Updated/added ACL between {user_record.title} and " - f"{parent_record.title}{bcolors.ENDC}") - except Exception as err: - print(f"{bcolors.FAIL}Could not update ACL: {err}{bcolors.ENDC}") diff --git a/keepercommander/commands/pam_debug/info.py b/keepercommander/commands/pam_debug/info.py index a89ddbae7..74ad5d3db 100644 --- a/keepercommander/commands/pam_debug/info.py +++ b/keepercommander/commands/pam_debug/info.py @@ -5,7 +5,7 @@ from . import load_pam_record from ...discovery_common.infrastructure import Infrastructure from ...discovery_common.record_link import RecordLink -from ...discovery_common.types import UserAcl, DiscoveryObject +from ...discovery_common.types import UserAcl, DiscoveryObject, ServiceEnum from ...discovery_common.constants import PAM_USER, PAM_MACHINE, PAM_DATABASE, PAM_DIRECTORY from ...keeper_dag import EdgeType import time @@ -16,6 +16,7 @@ if TYPE_CHECKING: from ...vault import TypedRecord from ...params import KeeperParams + from ...discovery_common.types import UserAclServiceNames, UserAclServiceNamesItem class PAMDebugInfoCommand(PAMGatewayActionDiscoverCommandBase): @@ -28,8 +29,18 @@ class PAMDebugInfoCommand(PAMGatewayActionDiscoverCommandBase): PAM_DIRECTORY: "PAM Directory", } + TITLES = { + ServiceEnum.service: "Service", + ServiceEnum.task: "Scheduled Task", + ServiceEnum.iis_pool: "IIS Pool", + ServiceEnum.com: "COM (Classic)", + ServiceEnum.dcom: "DCOM", + ServiceEnum.com_plus: "COM Plus", + ServiceEnum.scom: "SCOM", + } + # The record to base everything on. - parser.add_argument('--record-uid', '-i', required=True, dest='record_uid', action='store', + parser.add_argument('--record-uid', '-i', '-r', required=True, dest='record_uid', action='store', help='Keeper PAM record UID.') def get_parser(self): @@ -298,7 +309,7 @@ def _print_field(f): # Get the resource record machine_record = load_pam_record(params, - machine_vertex.uid) # type: TypedRecord | None + machine_vertex.uid) # type: TypedRecord | None # If the resource record does not exist. if machine_record is None: @@ -319,8 +330,18 @@ def _print_field(f): # Record exists; just use information from the record. else: - machines.append(f" * {machine_record.title}, {machine_record.record_uid}, " - f"vertex {machine_vertex.uid}") + text = f" * {machine_record.title}, {machine_record.record_uid}, "\ + f"vertex {machine_vertex.uid}" + if acl.service_names is not None: + for service_name in acl.service_names: # type: UserAclServiceNames + if len(service_name.items) > 0: + text += f"\n + {PAMDebugInfoCommand.TITLES.get(service_name.type)}\n" + for service_item in service_name.items: + text += f"\n . {service_item.name}" + if not service_item.via_discovery: + text += f" (manual entry)" + text += "\n" + machines.append(text) if len(machines) > 0: print(f"{bcolors.HEADER}Controls Services on Machine{bcolors.ENDC}") @@ -465,6 +486,35 @@ def _print_field(f): print(f" * {iis_pool.name} = {iis_pool.user}") else: print(" Machines has no IIS Pools that are using non-builtin users.") + + print(f" {self._b('COM (classic)')} (Non Builtin Users)") + if len(content.item.facts.coms) > 0: + for com in content.item.facts.coms: + print(f" * {com.name} = {com.user}") + else: + print(" Machines has no COM (classic) applications that are using non-builtin users.") + + print(f" {self._b('DCOM')} (Non Builtin Users)") + if len(content.item.facts.dcoms) > 0: + for dcom in content.item.facts.dcoms: + print(f" * {dcom.name} = {dcom.user}") + else: + print(" Machines has no DCOM applications that are using non-builtin users.") + + print(f" {self._b('COM Plus')} (Non Builtin Users)") + if len(content.item.facts.com_pluses) > 0: + for com in content.item.facts.com_pluses: + print(f" * {com.name} = {com.user}") + else: + print(" Machines has no COM Plus applications that are using non-builtin users.") + + print(f" {self._b('SCOM')} (Non Builtin Users)") + if len(content.item.facts.scoms) > 0: + for scom in content.item.facts.scoms: + print(f" * {scom.name} = {scom.user}") + else: + print(" Machines has no SCOM applications that are using non-builtin users.") + else: print(f"{bcolors.FAIL} Machine facts are not set. Discover inside may not have been " f"performed.{bcolors.ENDC}") diff --git a/keepercommander/commands/pam_debug/link.py b/keepercommander/commands/pam_debug/link.py deleted file mode 100644 index deae2697a..000000000 --- a/keepercommander/commands/pam_debug/link.py +++ /dev/null @@ -1,75 +0,0 @@ -from __future__ import annotations -import argparse -import logging -from ..discover import (PAMGatewayActionDiscoverCommandBase, GatewayContext, PAM_MACHINE, PAM_DATABASE, PAM_DIRECTORY, - MultiConfigurationException, multi_conf_msg) -from ...display import bcolors -from . import load_pam_record -from ...discovery_common.record_link import RecordLink -from typing import TYPE_CHECKING - -if TYPE_CHECKING: - from ...vault import TypedRecord - from ...params import KeeperParams - - -class PAMDebugLinkCommand(PAMGatewayActionDiscoverCommandBase): - parser = argparse.ArgumentParser(prog='pam action debug link') - - # The record to base everything on. - parser.add_argument('--gateway', '-g', required=True, dest='gateway', action='store', - help='Gateway name or UID.') - parser.add_argument('--configuration-uid', "-c", required=False, dest='configuration_uid', - action='store', help='PAM configuration UID, if gateway has multiple.') - - parser.add_argument('--resource-uid', '-r', required=True, dest='resource_uid', action='store', - help='Resource record UID.') - parser.add_argument('--debug-gs-level', required=False, dest='debug_level', action='store', - help='GraphSync debug level. Default is 0', type=int, default=0) - - def get_parser(self): - return PAMDebugLinkCommand.parser - - def execute(self, params: KeeperParams, **kwargs): - - gateway = kwargs.get("gateway") - resource_uid = kwargs.get("resource_uid") - debug_level = int(kwargs.get("debug_level", 0)) - - print("") - - configuration_uid = kwargs.get('configuration_uid') - try: - gateway_context = GatewayContext.from_gateway(params=params, - gateway=gateway, - configuration_uid=configuration_uid) - if gateway_context is None: - print(f"{bcolors.FAIL}Could not find the gateway configuration for {gateway}.{bcolors.ENDC}") - return - except MultiConfigurationException as err: - multi_conf_msg(gateway, err) - return - - record_link = RecordLink(record=gateway_context.configuration, - params=params, - logger=logging, - debug_level=debug_level, use_per_graph_endpoints=False) - - resource_record = load_pam_record(params, resource_uid) # type: TypedRecord | None - if resource_record is None: - print(f"{bcolors.FAIL}The parent record does not exists.{bcolors.ENDC}") - return - - if resource_record.record_type not in [PAM_MACHINE, PAM_DATABASE, PAM_DIRECTORY]: - print(f"{bcolors.FAIL}The resource record type, {resource_record.record_type} " - f"is not allowed.{bcolors.ENDC}") - return - - try: - record_link.belongs_to(resource_uid, gateway_context.configuration_uid, ) - record_link.save() - print(f"{bcolors.OKGREEN}Added link between '{resource_uid}' and " - f"{gateway_context.configuration_uid}{bcolors.ENDC}") - except Exception as err: - print(f"{bcolors.FAIL}Could not add LINK: {err}{bcolors.ENDC}") - raise err diff --git a/keepercommander/commands/pam_import/record_loader.py b/keepercommander/commands/pam_import/record_loader.py index 306614798..b24b26b0e 100644 --- a/keepercommander/commands/pam_import/record_loader.py +++ b/keepercommander/commands/pam_import/record_loader.py @@ -35,33 +35,43 @@ def iter_accessible_record_uids(params) -> Iterator[str]: def load_pam_record(params, record_uid: str) -> Optional[vault.KeeperRecord]: - """Load a vault record from classic cache, NSF cache, or NSF record data.""" + """Load a vault record from classic cache, NSF cache, or NSF record data. + + Priority: classic cache (highest revision) > NSF cache > NSF record data (lowest). + """ record_uid = (record_uid or '').strip() if not record_uid: return None + # Try classic cache first (most reliable, has encryption keys and metadata) 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 + # Try loading by UID from classic vault (may trigger decryption from cached key material) rec = vault.KeeperRecord.load(params, record_uid) if rec: return rec + # Fall back to NSF cache (record_key=None, may be stale, but better than nothing) 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 + if 'data_json' in rd: + dj = rd['data_json'] + version = nsf_records.get(record_uid, {}).get('version', 3) + if version in (3, 6): + keeper_record = vault.TypedRecord(version=version) + keeper_record.record_uid = record_uid + keeper_record.load_record_data(dj, None) + return keeper_record - dj = rd['data_json'] - version = nsf_records.get(record_uid, {}).get('version', 3) - if version not in (3, 6): - 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 - keeper_record = vault.TypedRecord(version=version) - keeper_record.record_uid = record_uid - keeper_record.load_record_data(dj, None) - return keeper_record + return vault.KeeperRecord.load(params, record_uid) diff --git a/keepercommander/commands/pam_saas/config.py b/keepercommander/commands/pam_saas/config.py index b27eff20e..f40b151f4 100644 --- a/keepercommander/commands/pam_saas/config.py +++ b/keepercommander/commands/pam_saas/config.py @@ -181,6 +181,7 @@ def _create_config(params: KeeperParams, plugin_code_bytes: bytes | None = None): from ..pam.vault_target import ( create_record_in_folder, update_pam_record, is_nested_share_folder, + reload_pam_record_if_nsf_updated, ) from ..pam_import.nsf_helpers import sync_down_preserving_nsf_keys @@ -269,7 +270,8 @@ def _create_config(params: KeeperParams, attachment.upload_attachments(params, existing_record, [task]) # upload_attachments updates fileRef on existing_record via facade - update_pam_record(params, existing_record, command='pam action saas config') + was_nsf = update_pam_record(params, existing_record, command='pam action saas config') + existing_record = reload_pam_record_if_nsf_updated(params, existing_record, existing_record.record_uid, was_nsf) print("") print(f"{bcolors.OKGREEN}Created SaaS configuration record with UID of {record.record_uid}{bcolors.ENDC}") diff --git a/keepercommander/commands/pam_service/add.py b/keepercommander/commands/pam_service/add.py index 05c9ec055..079d15fa4 100644 --- a/keepercommander/commands/pam_service/add.py +++ b/keepercommander/commands/pam_service/add.py @@ -32,7 +32,15 @@ class PAMActionServiceAddCommand(PAMGatewayActionDiscoverCommandBase): parser.add_argument('--user-uid', '-u', required=True, dest='user_uid', action='store', help='The UID of the User record') parser.add_argument('--type', '-t', required=True, dest='service_type', action='store', - choices=["service", "task", "iis_pool"], + choices=[ + "service", + "task", + "iis_pool", + "com", + "dcom", + "com_plus", + "scom" + ], help='Type of service.') parser.add_argument('--name', '-n', required=True, dest='name', action='store', help='Name label for reporting.') diff --git a/keepercommander/commands/pam_service/list.py b/keepercommander/commands/pam_service/list.py index c74cd4c6e..7b61459a6 100644 --- a/keepercommander/commands/pam_service/list.py +++ b/keepercommander/commands/pam_service/list.py @@ -28,6 +28,16 @@ class PAMActionServiceListCommand(PAMGatewayActionDiscoverCommandBase): parser.add_argument('--by-machine', '-m', required=False, dest='do_by_machine', action='store_true', help='List by machine') + TITLES = { + ServiceEnum.service: "Service", + ServiceEnum.task: "Scheduled Task", + ServiceEnum.iis_pool: "IIS Pool", + ServiceEnum.com: "COM (Classic)", + ServiceEnum.dcom: "DCOM", + ServiceEnum.com_plus: "COM Plus", + ServiceEnum.scom: "SCOM", + } + def get_parser(self): return PAMActionServiceListCommand.parser @@ -76,13 +86,7 @@ def _by_user(self, params: KeeperParams, record_link: RecordLink, user_service: items = [] if acl.service_names is not None or acl.service_names != "": for service_name in acl.get_service_names(user_record.record_key): - text = "" - if service_name.type == ServiceEnum.service: - text = "Service" - elif service_name.type == ServiceEnum.task: - text = "Scheduled Task" - elif service_name.type == ServiceEnum.iis_pool: - text = "IIS Pool" + text = PAMActionServiceListCommand.TITLES.get(service_name.type) for item in service_name.items: text += f": {item.name}" if "Unknown" in item.name: @@ -159,13 +163,7 @@ def _by_machine(self, params: KeeperParams, record_link: RecordLink, user_servic items = [] if acl.service_names is not None or acl.service_names != "": for service_name in acl.get_service_names(user_record.record_key): - text = "" - if service_name.type == ServiceEnum.service: - text = "Service" - elif service_name.type == ServiceEnum.task: - text = "Scheduled Task" - elif service_name.type == ServiceEnum.iis_pool: - text = "IIS Pool" + text = PAMActionServiceListCommand.TITLES.get(service_name.type) for item in service_name.items: text += f": {item.name}" if "Unknown" in item.name: diff --git a/keepercommander/commands/pam_service/remove.py b/keepercommander/commands/pam_service/remove.py index f66da90e0..0451eb0cb 100644 --- a/keepercommander/commands/pam_service/remove.py +++ b/keepercommander/commands/pam_service/remove.py @@ -31,8 +31,17 @@ class PAMActionServiceRemoveCommand(PAMGatewayActionDiscoverCommandBase): help='The UID of the User record') parser.add_argument('--type', '-t', required=True, dest='service_type', action='store', - choices=["service", "task", "iis_pool", "all"], - help='Type of service. "all" will clear all') + choices=[ + "all", + "service", + "task", + "iis_pool", + "com", + "dcom", + "com_plus", + "scom" + ], + help='Type of service. "all" will clear all.') parser.add_argument('--name', '-n', required=False, dest='name', action='store', help='Name label for reporting. Exclude will remove all service types.') diff --git a/keepercommander/commands/record.py b/keepercommander/commands/record.py index 1774a65d5..a66ab6f3c 100644 --- a/keepercommander/commands/record.py +++ b/keepercommander/commands/record.py @@ -1882,15 +1882,21 @@ def execute(self, params, **kwargs): fmt = kwargs.get('format', 'table') pattern = kwargs['pattern'] if 'pattern' in kwargs else None results = api.search_shared_folders(params, pattern or '') + nsf_results = [] + if kwargs.get('roe_eligible'): results = [sf for sf in results if vault_extensions.shared_folder_has_pam_user_with_rotation(params, sf.shared_folder_uid)] - if any(results): - table = [] - headers = ['shared_folder_uid', 'name'] if fmt == 'json' else ['Shared Folder UID', 'Name'] - for sf in results: - row = [sf.shared_folder_uid, sf.name] - table.append(row) + + nsf_results = api.search_nested_share_folders(params, pattern or '') + nsf_results = [(folder_uid, name) for folder_uid, name in nsf_results + if vault_extensions.nested_share_folder_has_pam_user_with_rotation(params, folder_uid)] + + table = [[sf.shared_folder_uid, sf.name, 'Classic'] for sf in results] + table.extend([[folder_uid, name, 'Nested'] for folder_uid, name in nsf_results]) + + if table: + headers = ['shared_folder_uid', 'name', 'folder_type'] if fmt == 'json' else ['Shared Folder UID', 'Name', 'Type'] table.sort(key=lambda x: (x[1] or '').lower()) return base.dump_report_data(table, headers, fmt=fmt, filename=kwargs.get('output'), diff --git a/keepercommander/commands/register.py b/keepercommander/commands/register.py index 4c3222e6b..73b839576 100644 --- a/keepercommander/commands/register.py +++ b/keepercommander/commands/register.py @@ -807,7 +807,9 @@ def apply_share_expiration(target): uo.typedSharedFolderKey.encryptedKeyType = folder_pb2.encrypted_by_public_key rq.sharedFolderAddUser.append(uo) - else: + elif not invited: + # After a successful invite, keys are unavailable until accepted. + # Do not emit "User not found" — Service Mode treats that as failure. logging.warning('User %s not found', email) if len(teams) > 0: diff --git a/keepercommander/commands/tunnel_and_connections.py b/keepercommander/commands/tunnel_and_connections.py index caf45a35f..f818ef8f7 100644 --- a/keepercommander/commands/tunnel_and_connections.py +++ b/keepercommander/commands/tunnel_and_connections.py @@ -36,7 +36,8 @@ 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 .pam.vault_target import update_pam_record, reload_pam_record_if_nsf_updated +from .pam_import.nsf_helpers import sync_down_preserving_nsf_keys from .tunnel_registry import ( PARENT_GRACE_SECONDS, is_pid_alive, @@ -48,6 +49,7 @@ from .. import api, vault from ..display import bcolors from ..error import CommandError +import json from ..params import LAST_RECORD_UID from ..subfolder import find_folders from ..utils import value_to_boolean @@ -394,6 +396,9 @@ def get_parser(self): return PAMTunnelEditCommand.pam_cmd_parser def execute(self, params, **kwargs): + # Ensure cache is fresh to avoid stale data from concurrent nsf-record-update calls + sync_down_preserving_nsf_keys(params) + tunneling_override_port = kwargs.get('tunneling_override_port') if ((kwargs.get('enable_tunneling') and kwargs.get('disable_tunneling')) or @@ -414,12 +419,16 @@ def execute(self, params, **kwargs): record_name = kwargs.get('record') if not record_name: raise CommandError('pam tunnel edit', '"record" parameter is required.') + + # First resolve to get record_uid record = RecordMixin.resolve_single_record(params, record_name) if not record: raise CommandError('pam tunnel edit', f'{bcolors.FAIL}Record \"{record_name}\" not found.{bcolors.ENDC}') if not isinstance(record, vault.TypedRecord): raise CommandError('pam tunnel edit', f'Record \"{record_name}\" can not be edited.') + record_uid = record.record_uid + # config parameter is optional and maybe (auto)resolved from PAM record config_name = kwargs.get('config', None) cfg_rec = RecordMixin.resolve_single_record(params, config_name) @@ -460,7 +469,8 @@ def execute(self, params, **kwargs): record.custom.append(record_seed) dirty = True if dirty: - update_pam_record(params, record, command='pam tunnel edit') + was_nsf = update_pam_record(params, record, command='pam tunnel edit') + record = reload_pam_record_if_nsf_updated(params, record, record_uid, was_nsf) traffic_encryption_key = record.get_typed_field('trafficEncryptionSeed') if not traffic_encryption_key: @@ -532,7 +542,8 @@ def execute(self, params, **kwargs): dirty = True # Persist the record changes (new pamSettings field or port modifications) if dirty: - update_pam_record(params, record, command='pam tunnel edit') + was_nsf = update_pam_record(params, record, command='pam tunnel edit') + record = reload_pam_record_if_nsf_updated(params, record, record_uid, was_nsf) 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. " @@ -582,12 +593,17 @@ def execute(self, params, **kwargs): if dirty: tmp_dag.set_resource_allowed(resource_uid=record_uid, tunneling=_tunneling, allowed_settings_name=allowed_settings_name) - update_pam_record(params, record, command='pam tunnel edit') + was_nsf = update_pam_record(params, record, command='pam tunnel edit') + record = reload_pam_record_if_nsf_updated(params, record, record_uid, was_nsf) # Print out the tunnel settings if not kwargs.get('silent'): tmp_dag.print_tunneling_config(record_uid, record.get_typed_field('pamSettings'), config_uid) + # Final sync ensures NSF record updates are fully propagated. (First sync occurs within + # update_pam_record for NSF updates; this is a belt-and-suspenders safety flush.) + sync_down_preserving_nsf_keys(params) + class PAMTunnelStartCommand(Command): pam_cmd_parser = argparse.ArgumentParser(prog='pam tunnel start') @@ -2711,6 +2727,9 @@ def get_parser(self): return PAMConnectionEditCommand.parser def execute(self, params, **kwargs): + # Ensure cache is fresh to avoid stale data from concurrent nsf-record-update calls + sync_down_preserving_nsf_keys(params) + connection_override_port = kwargs.get('connections_override_port', None) # Convert on/off/default to True/False/None @@ -2727,12 +2746,16 @@ def execute(self, params, **kwargs): record_name = kwargs.get('record') if not record_name: raise CommandError('pam connection edit', 'Record parameter is required.') + + # First resolve to get record_uid record = RecordMixin.resolve_single_record(params, record_name) if not record: raise CommandError('pam connection edit', f'{bcolors.FAIL}Record \"{record_name}\" not found.{bcolors.ENDC}') if not isinstance(record, vault.TypedRecord): raise CommandError('pam connection edit', f'Record \"{record_name}\" can not be edited.') + record_uid = record.record_uid + # config parameter is optional and maybe (auto)resolved from PAM record config_name = kwargs.get('config', None) cfg_rec = RecordMixin.resolve_single_record(params, config_name) @@ -2980,7 +3003,8 @@ def _get_effective_protocol(): logging.debug(f'security is already {target_sec} on record={record_uid}') if dirty: - update_pam_record(params, record, command='pam connection edit') + was_nsf = update_pam_record(params, record, command='pam connection edit') + record = reload_pam_record_if_nsf_updated(params, record, record_uid, was_nsf) traffic_encryption_key = record.get_typed_field('trafficEncryptionSeed') if not traffic_encryption_key: @@ -3145,6 +3169,10 @@ def _get_effective_protocol(): # Print out PAM Settings if not kwargs.get("silent", False): tdag.print_tunneling_config(record_uid, record.get_typed_field('pamSettings'), config_uid) + # Final sync ensures NSF record updates are fully propagated. (First sync occurs within + # update_pam_record for NSF updates; this is a belt-and-suspenders safety flush.) + sync_down_preserving_nsf_keys(params) + class PAMConnectionJitCommand(Command): parser = argparse.ArgumentParser(prog='pam connection jit') @@ -3760,6 +3788,9 @@ def get_parser(self): return PAMRbiEditCommand.parser def execute(self, params, **kwargs): + # Ensure cache is fresh to avoid stale data from concurrent nsf-record-update calls + sync_down_preserving_nsf_keys(params) + record_name = kwargs.get('record') or '' config_name = kwargs.get('config') or '' autofill = kwargs.get('autofill') or '' @@ -4051,7 +4082,8 @@ def update_connection_choice(field_name, value): update_connection_choice('sessionPersistence', session_persistence) if dirty: - update_pam_record(params, record, command='pam rbi edit') + was_nsf = update_pam_record(params, record, command='pam rbi edit') + record = reload_pam_record_if_nsf_updated(params, record, record_uid, was_nsf) traffic_encryption_key = record.get_typed_field('trafficEncryptionSeed') if not traffic_encryption_key: @@ -4154,7 +4186,9 @@ def update_connection_choice(field_name, value): session_recording=rec_val) # if not kwargs.get("silent", False): # tdag.print_tunneling_config(record_uid, record.get_typed_field('pamRemoteBrowserSettings'), config_uid) - params.sync_data = True + # Final sync ensures NSF record updates are fully propagated. (First sync occurs within + # update_pam_record for NSF updates; this is a belt-and-suspenders safety flush.) + sync_down_preserving_nsf_keys(params) class PAMSplitCommand(Command): pam_cmd_parser = argparse.ArgumentParser(prog='pam split') @@ -4229,7 +4263,8 @@ def execute(self, params, **kwargs): pam_settings = vault.TypedField.new_field('pamSettings', "", "") record.fields.append(pam_settings) - update_pam_record(params, record, command='pam-split') + was_nsf = update_pam_record(params, record, command='pam-split') + record = reload_pam_record_if_nsf_updated(params, record, record_uid, was_nsf) print(f"{bcolors.WARNING}Record {record_uid} has no data to split and " "was converted to the new format. Remember to manually add " @@ -4284,7 +4319,8 @@ def execute(self, params, **kwargs): pam_settings = vault.TypedField.new_field('pamSettings', "", "") record.fields.append(pam_settings) - update_pam_record(params, record, command='pam-split') + was_nsf = update_pam_record(params, record, command='pam-split') + record = reload_pam_record_if_nsf_updated(params, record, record_uid, was_nsf) if pam_config_uid: encrypted_session_token, encrypted_transmission_key, transmission_key = get_keeper_tokens(params) diff --git a/keepercommander/commands/universalsecretsync.py b/keepercommander/commands/universalsecretsync.py index 5bbda109e..2cdde30f5 100644 --- a/keepercommander/commands/universalsecretsync.py +++ b/keepercommander/commands/universalsecretsync.py @@ -79,7 +79,7 @@ def print_uss_configurations_list(params, format_type='table'): # Only process these specific configuration types uss_supported_types = ('pamGcpConfiguration', 'pamAzureConfiguration', 'pamAwsConfiguration', - 'pamGitHubConfiguration') + 'pamGitHubConfiguration', 'pamHashiCorpConfiguration') configs_data = [] for record in configurations: @@ -115,6 +115,21 @@ def print_uss_configurations_list(params, format_type='table'): # GitHub-specific fields are nested under the 'github' key. github_data = config_data.get('github') or {} + # HashiCorp-specific fields are nested under the 'hashicorp' key. + hashicorp_data = config_data.get('hashicorp') or {} + + # Decrypt HashiCorp vault_base_url if present + vault_base_url = 'N/A' + vault_base_url_encrypted = hashicorp_data.get('vaultBaseUrl') + if vault_base_url_encrypted: + try: + vault_base_url_bytes = crypto.decrypt_aes_v2( + utils.base64_url_decode(vault_base_url_encrypted), record.record_key) + vault_base_url = vault_base_url_bytes.decode('utf-8') + except Exception as e: + logging.debug(f"Failed to decrypt vault_base_url for record {record.record_uid}: {e}") + vault_base_url = 'N/A' + # Decrypt vault_name if present. The router stores it under the # 'vaultName' key as a base64-url string of the encrypted bytes. vault_name = 'N/A' @@ -206,6 +221,7 @@ def print_uss_configurations_list(params, format_type='table'): 'owner': owner, 'organization_visibility': org_visibility_str, 'repos': repo_names, + 'vault_base_url': vault_base_url, }) except Exception as e: # Skip records that fail to load or don't have USS config @@ -227,7 +243,7 @@ def print_uss_configurations_list(params, format_type='table'): # Display as simple summary table table = [] headers = ['Network UID', 'Title', 'Type', 'Enabled', 'Dry Run', 'Folders', 'Vault Name', 'Sync Identity', - 'Scope', 'Owner', 'Org Visibility', 'Repos'] + 'Scope', 'Owner', 'Org Visibility', 'Repos', 'Vault Base URL'] for config in configs_data: enabled_str = f"{bcolors.OKGREEN}Yes{bcolors.ENDC}" if config['enabled'] else f"{bcolors.FAIL}No{bcolors.ENDC}" @@ -249,7 +265,8 @@ def print_uss_configurations_list(params, format_type='table'): config.get('scope', 'N/A'), config.get('owner', 'N/A'), config.get('organization_visibility', 'N/A'), - repos_str + repos_str, + config.get('vault_base_url', 'N/A') ] table.append(row) @@ -273,7 +290,7 @@ def print_uss_configuration_details(params, network_uid, format_type='table'): # Check if it's a supported USS configuration type uss_supported_types = ('pamGcpConfiguration', 'pamAzureConfiguration', 'pamAwsConfiguration', - 'pamGitHubConfiguration') + 'pamGitHubConfiguration', 'pamHashiCorpConfiguration') if not isinstance(network, vault.TypedRecord) or network.record_type not in uss_supported_types: if format_type == 'json': return json.dumps({"error": f'Record "{network_uid}" is not a USS configuration'}) @@ -310,6 +327,21 @@ def print_uss_configuration_details(params, network_uid, format_type='table'): # GitHub-specific fields are nested under the 'github' key. github_data = config_data.get('github') or {} + # HashiCorp-specific fields are nested under the 'hashicorp' key. + hashicorp_data = config_data.get('hashicorp') or {} + + # Decrypt HashiCorp vault_base_url if present + vault_base_url = 'N/A' + vault_base_url_encrypted = hashicorp_data.get('vaultBaseUrl') + if vault_base_url_encrypted: + try: + vault_base_url_bytes = crypto.decrypt_aes_v2( + utils.base64_url_decode(vault_base_url_encrypted), network.record_key) + vault_base_url = vault_base_url_bytes.decode('utf-8') + except Exception as e: + logging.debug(f"Failed to decrypt vault_base_url for network {network.record_uid}: {e}") + vault_base_url = 'N/A' + # Decrypt vault_name if present. The router stores it under the # 'vaultName' key as a base64-url string of the encrypted bytes. vault_name = 'N/A' @@ -438,6 +470,7 @@ def print_uss_configuration_details(params, network_uid, format_type='table'): 'owner': owner, 'organization_visibility': org_visibility_str, 'repos': repo_names, + 'vault_base_url': vault_base_url, 'folders': [] } @@ -473,6 +506,7 @@ def print_uss_configuration_details(params, network_uid, format_type='table'): table.append(['Owner', owner]) table.append(['Org Visibility', org_visibility_str]) table.append(['Repos', ', '.join(repo_names) if repo_names else 'None']) + table.append(['Vault Base URL', vault_base_url]) table.append(['', '']) # Blank row separator # Display folder sync details @@ -521,6 +555,7 @@ def print_uss_configuration_details(params, network_uid, format_type='table'): class PAMUniversalSyncConfigAddCommand(Command): parser = argparse.ArgumentParser(prog='pam universal-sync-config add') + parser.add_argument('--network', '-n', required=True, dest='network', action='store', help='Network UID or name to configure universal sync') parser.add_argument('--enabled', '-e', dest='enabled', action='store', @@ -543,6 +578,14 @@ class PAMUniversalSyncConfigAddCommand(Command): help='Repository visibility to sync when scope is organization') parser.add_argument('--repo', '-r', dest='repo', action='append', help='GitHub repository name to sync (can be specified multiple times; scope must be repository)') + parser.add_argument('--vault-base-url', '-vbu', dest='vault_base_url', action='store', + help='HashiCorp Vault Base URL (e.g., https://vault.company.com:8200)') + parser.add_argument('--vault-token', '-vt', dest='vault_token', action='store', + help='HashiCorp Vault Token (optional; syncIdentity takes precedence)') + parser.add_argument('--vault-namespace', '-vns', dest='vault_namespace', action='store', + help='HashiCorp Vault Namespace (optional; leave blank for Community Edition)') + parser.add_argument('--vault-mount-path', '-vmp', dest='vault_mount_path', action='store', + help='HashiCorp Vault KV Mount Path (optional; defaults to "secret")') def get_parser(self): return PAMUniversalSyncConfigAddCommand.parser @@ -616,6 +659,30 @@ def execute(self, params, **kwargs): repo_obj.name = crypto.encrypt_aes_v2(repo_bytes, network.record_key) rq.github.repos.append(repo_obj) + vault_base_url = kwargs.get('vault_base_url') + if vault_base_url: + vault_base_url_bytes = string_to_bytes(vault_base_url) + encrypted_vault_base_url = crypto.encrypt_aes_v2(vault_base_url_bytes, network.record_key) + rq.hashicorp.vaultBaseUrl = encrypted_vault_base_url + + vault_token = kwargs.get('vault_token') + if vault_token: + vault_token_bytes = string_to_bytes(vault_token) + encrypted_vault_token = crypto.encrypt_aes_v2(vault_token_bytes, network.record_key) + rq.hashicorp.vaultToken = encrypted_vault_token + + vault_namespace = kwargs.get('vault_namespace') + if vault_namespace: + vault_namespace_bytes = string_to_bytes(vault_namespace) + encrypted_vault_namespace = crypto.encrypt_aes_v2(vault_namespace_bytes, network.record_key) + rq.hashicorp.vaultNamespace = encrypted_vault_namespace + + vault_mount_path = kwargs.get('vault_mount_path') + if vault_mount_path: + vault_mount_path_bytes = string_to_bytes(vault_mount_path) + encrypted_vault_mount_path = crypto.encrypt_aes_v2(vault_mount_path_bytes, network.record_key) + rq.hashicorp.vaultMountPath = encrypted_vault_mount_path + encrypted_session_token, encrypted_transmission_key, transmission_key = get_keeper_tokens(params) try: @@ -651,6 +718,14 @@ class PAMUniversalSyncConfigEditCommand(Command): help='Repository visibility to sync when scope is organization') parser.add_argument('--repo', '-r', dest='repo', action='append', help='GitHub repository name to sync (can be specified multiple times; scope must be repository)') + parser.add_argument('--vault-base-url', '-vbu', dest='vault_base_url', action='store', + help='HashiCorp Vault Base URL (e.g., https://vault.company.com:8200)') + parser.add_argument('--vault-token', '-vt', dest='vault_token', action='store', + help='HashiCorp Vault Token (optional; syncIdentity takes precedence)') + parser.add_argument('--vault-namespace', '-vns', dest='vault_namespace', action='store', + help='HashiCorp Vault Namespace (optional; leave blank for Community Edition)') + parser.add_argument('--vault-mount-path', '-vmp', dest='vault_mount_path', action='store', + help='HashiCorp Vault KV Mount Path (optional; defaults to "secret")') def get_parser(self): return PAMUniversalSyncConfigEditCommand.parser @@ -786,6 +861,41 @@ def execute(self, params, **kwargs): repo_obj.name = utils.base64_url_decode(existing_repo) rq.github.repos.append(repo_obj) + # HashiCorp-specific fields live under the nested 'hashicorp' object + existing_hashicorp = existing_config.get('hashicorp') or {} + + vault_base_url = kwargs.get('vault_base_url') + if vault_base_url: + vault_base_url_bytes = string_to_bytes(vault_base_url) + encrypted_vault_base_url = crypto.encrypt_aes_v2(vault_base_url_bytes, network.record_key) + rq.hashicorp.vaultBaseUrl = encrypted_vault_base_url + elif existing_hashicorp.get('vaultBaseUrl'): + rq.hashicorp.vaultBaseUrl = utils.base64_url_decode(existing_hashicorp['vaultBaseUrl']) + + vault_token = kwargs.get('vault_token') + if vault_token: + vault_token_bytes = string_to_bytes(vault_token) + encrypted_vault_token = crypto.encrypt_aes_v2(vault_token_bytes, network.record_key) + rq.hashicorp.vaultToken = encrypted_vault_token + elif existing_hashicorp.get('vaultToken'): + rq.hashicorp.vaultToken = utils.base64_url_decode(existing_hashicorp['vaultToken']) + + vault_namespace = kwargs.get('vault_namespace') + if vault_namespace: + vault_namespace_bytes = string_to_bytes(vault_namespace) + encrypted_vault_namespace = crypto.encrypt_aes_v2(vault_namespace_bytes, network.record_key) + rq.hashicorp.vaultNamespace = encrypted_vault_namespace + elif existing_hashicorp.get('vaultNamespace'): + rq.hashicorp.vaultNamespace = utils.base64_url_decode(existing_hashicorp['vaultNamespace']) + + vault_mount_path = kwargs.get('vault_mount_path') + if vault_mount_path: + vault_mount_path_bytes = string_to_bytes(vault_mount_path) + encrypted_vault_mount_path = crypto.encrypt_aes_v2(vault_mount_path_bytes, network.record_key) + rq.hashicorp.vaultMountPath = encrypted_vault_mount_path + elif existing_hashicorp.get('vaultMountPath'): + rq.hashicorp.vaultMountPath = utils.base64_url_decode(existing_hashicorp['vaultMountPath']) + encrypted_session_token, encrypted_transmission_key, transmission_key = get_keeper_tokens(params) try: diff --git a/keepercommander/discovery_common/__version__.py b/keepercommander/discovery_common/__version__.py index b46c29a4b..5eba4ad50 100644 --- a/keepercommander/discovery_common/__version__.py +++ b/keepercommander/discovery_common/__version__.py @@ -1 +1 @@ -__version__ = '1.1.22' +__version__ = '1.1.23' diff --git a/keepercommander/discovery_common/types.py b/keepercommander/discovery_common/types.py index 04ecbae81..1ba1eafc8 100644 --- a/keepercommander/discovery_common/types.py +++ b/keepercommander/discovery_common/types.py @@ -358,6 +358,9 @@ class ServiceEnum(BaseEnum): task = "task" iis_pool = "iis_pool" dcom = "dcom" + com = "com" + com_plus = "com_plus" + scom = "scom" class UserAclServiceNames(BaseModel): @@ -550,7 +553,10 @@ class Facts(BaseModel): services: List[FactsNameUser] = [] tasks: List[FactsNameUser] = [] iis_pools: List[FactsNameUser] = [] + coms: List[FactsNameUser] = [] dcoms: List[FactsNameUser] = [] + com_pluses: List[FactsNameUser] = [] + scoms: List[FactsNameUser] = [] @property def has_services(self): @@ -564,13 +570,31 @@ def has_tasks(self): def has_iis_pools(self): return self.iis_pools is not None and len(self.iis_pools) > 0 + @property + def has_coms(self): + return self.coms is not None and len(self.coms) > 0 + @property def has_dcoms(self): return self.dcoms is not None and len(self.dcoms) > 0 + @property + def has_com_pluses(self): + return self.com_pluses is not None and len(self.com_pluses) > 0 + + @property + def has_scoms(self): + return self.scoms is not None and len(self.scoms) > 0 + @property def has_service_items(self): - return self.has_services or self.has_tasks or self.has_iis_pools or self.has_dcoms + return self.has_services \ + or self.has_tasks \ + or self.has_iis_pools \ + or self.has_coms \ + or self.has_dcoms \ + or self.has_com_pluses \ + or self.has_scoms class DiscoveryMachine(DiscoveryItem): diff --git a/keepercommander/discovery_common/user_service.py b/keepercommander/discovery_common/user_service.py index 328903561..619f49e60 100644 --- a/keepercommander/discovery_common/user_service.py +++ b/keepercommander/discovery_common/user_service.py @@ -858,14 +858,19 @@ def _connect_users_to_machine_services(self, self.debug(f" > {k} = {v}") # Add mapping from user to machine, that control services. - for service_type in [ServiceEnum.service, ServiceEnum.task, ServiceEnum.iis_pool]: + for service_type in [ServiceEnum.service, ServiceEnum.task, ServiceEnum.iis_pool, + ServiceEnum.com, ServiceEnum.dcom, ServiceEnum.com_plus, ServiceEnum.scom]: self.debug("-" * 40) self.debug(f"processing {service_type.value}s for {infra_machine_content.name} " f"({infra_machine_vertex.uid})") # Get the pair of name of the service and the user that controls it. # This is from discovery. - service_pairs = getattr(infra_machine_content.item.facts, f"{service_type.value}s") + if hasattr(infra_machine_content.item.facts, f"{service_type.value}s"): + service_pairs = getattr(infra_machine_content.item.facts, f"{service_type.value}s") + else: + service_pairs = getattr(infra_machine_content.item.facts, f"{service_type.value}es") + if len(service_pairs) == 0: self.debug(" no users control this type of service, skipping") continue diff --git a/keepercommander/importer/cyberark/cyberark.py b/keepercommander/importer/cyberark/cyberark.py index c66a2bd9f..2873bf209 100644 --- a/keepercommander/importer/cyberark/cyberark.py +++ b/keepercommander/importer/cyberark/cyberark.py @@ -28,6 +28,7 @@ BaseDownloadMembership, BaseImporter, Folder, + PathDelimiter, Permission, Record, RecordField, diff --git a/keepercommander/nested_share_folder/__init__.py b/keepercommander/nested_share_folder/__init__.py index b2d656c99..607c53a90 100644 --- a/keepercommander/nested_share_folder/__init__.py +++ b/keepercommander/nested_share_folder/__init__.py @@ -22,7 +22,7 @@ 'get_record_from_cache', 'get_record_revision', 'patch_record_revision', 'parse_sharing_status', 'get_record_key_type', 'encrypt_record_key_for_folder', 'encrypt_for_recipient', - 'handle_share_invite', 'resolve_user_uid_bytes', + 'handle_share_invite', 'ShareInviteSentError', 'resolve_user_uid_bytes', 'load_user_public_key', 'parse_folder_access_result', 'resolve_team_uid_bytes', 'resolve_team_identifier', 'get_team_keys', 'encrypt_for_team', 'is_keeper_uid', diff --git a/keepercommander/nested_share_folder/common.py b/keepercommander/nested_share_folder/common.py index 9b0f4c387..64db3b199 100644 --- a/keepercommander/nested_share_folder/common.py +++ b/keepercommander/nested_share_folder/common.py @@ -514,8 +514,16 @@ def _retry_with_canonical_email(params, recipient_email, _load_pk, # Share invite helper (previously duplicated in share + update_share) # ═══════════════════════════════════════════════════════════════════════════ +class ShareInviteSentError(ValueError): + """Invite was sent; caller should treat as success-with-notice.""" + + def handle_share_invite(params, recipient_email, needs_invite): - """Send a share invite if *needs_invite* is True; raise ValueError.""" + """Send a share invite if *needs_invite* is True. + + Raises ShareInviteSentError after a successful invite (subclass of ValueError). + Raises ValueError when the invite could not be sent. + """ if not needs_invite: return try: @@ -523,10 +531,10 @@ def handle_share_invite(params, recipient_email, needs_invite): rq = APIRequest_pb2.SendShareInviteRequest() rq.email = recipient_email api.communicate_rest(params, rq, 'vault/send_share_invite') - raise ValueError( + raise ShareInviteSentError( f"Share invitation has been sent to '{recipient_email}'. " f"Please repeat this command once the invitation is accepted.") - except ValueError: + except ShareInviteSentError: raise except Exception: raise ValueError( diff --git a/keepercommander/nested_share_folder/record_api.py b/keepercommander/nested_share_folder/record_api.py index 66c1dda35..4f2227638 100644 --- a/keepercommander/nested_share_folder/record_api.py +++ b/keepercommander/nested_share_folder/record_api.py @@ -283,6 +283,14 @@ def _build_update(payload, rev): new_revision = getattr(response, 'revision', 0) if success: patch_record_revision(params, record_uid, new_revision) + nsf_data = getattr(params, 'nested_share_record_data', {}).get(record_uid) + if nsf_data is not None: + # Update NSF cache with decrypted record data (dict form) + nsf_data['data_json'] = data + cached_record = getattr(params, 'record_cache', {}).get(record_uid) + if cached_record is not None and cached_record.get('data_unencrypted') is not None: + # Update classic cache with decrypted record data (JSON string form) + cached_record['data_unencrypted'] = json.dumps(data) return { 'record_uid': record_uid, 'status': record_pb2.RecordModifyResult.Name(r.status), diff --git a/keepercommander/scim/data_sources.py b/keepercommander/scim/data_sources.py index 866b433a4..9f872fdfb 100644 --- a/keepercommander/scim/data_sources.py +++ b/keepercommander/scim/data_sources.py @@ -80,14 +80,17 @@ def get_ldap_connection(self): logging.debug('AD connect: Kerberos auth method. Requires Windows, domain user, and domain computer') auth_method = ldap3.SASL server = ldap3.Server(self.ad_url, tls=tls) - with ldap3.Connection(server, user=self.ad_user, password=self.ad_password, + with ldap3.Connection(server, raise_exceptions=True, + user=self.ad_user, password=self.ad_password, authentication=auth_method, sasl_mechanism=ldap3.KERBEROS if auth_method == ldap3.SASL else None) as connection: - connection.open() - connection.bind() - if not connection.bind(): - raise Exception('Invalid AD username or password') - yield connection + try: + connection.open() + if not connection.bind(): + raise Exception('Invalid AD username or password') + yield connection + finally: + connection.unbind() def _build_domain_lookup(self, connection) -> Dict[str, str]: """Build a mapping of DNS domain names to NetBIOS names. diff --git a/keepercommander/service/api/command.py b/keepercommander/service/api/command.py index c80c6b1ea..bb736bec7 100644 --- a/keepercommander/service/api/command.py +++ b/keepercommander/service/api/command.py @@ -99,7 +99,7 @@ def execute_command_direct(**kwargs) -> Tuple[Union[Response, bytes], int]: ) return response_data - response, status_code = CommandExecutor.execute(processed_command) + response, status_code = CommandExecutor.execute(processed_command, temp_files=temp_files) # If we get a busy response, add v1-specific message if (isinstance(response, dict) and diff --git a/keepercommander/service/core/request_queue.py b/keepercommander/service/core/request_queue.py index 44e387e19..175947418 100644 --- a/keepercommander/service/core/request_queue.py +++ b/keepercommander/service/core/request_queue.py @@ -325,7 +325,7 @@ def _process_request(self, request: QueuedRequest): try: # Execute the command using existing CommandExecutor - result, status_code = CommandExecutor.execute(request.command) + result, status_code = CommandExecutor.execute(request.command, temp_files=request.temp_files) # Mark as completed request.status = RequestStatus.COMPLETED diff --git a/keepercommander/service/util/command_util.py b/keepercommander/service/util/command_util.py index cb7d94bf6..0c7507f08 100644 --- a/keepercommander/service/util/command_util.py +++ b/keepercommander/service/util/command_util.py @@ -148,13 +148,12 @@ def _finalize_parsed_response(cls, response: Any) -> Tuple[Any, int]: return response, status_code @classmethod - def execute(cls, command: str) -> Tuple[Any, int]: + def execute(cls, command: str, temp_files: Optional[list] = None) -> Tuple[Any, int]: logger.debug(f"Executing command: {sanitize_command_fields(command)}") - validation_error = cls.validate_command(command) if validation_error: return validation_error - + from ..core.globals import ensure_params_loaded try: params = ensure_params_loaded() @@ -169,11 +168,19 @@ def execute(cls, command: str) -> Tuple[Any, int]: except ValueError: command_tokens = command.split() + # This request's own FILEDATA directory - the only paths Service + # Mode will treat as safe, not the whole shared OS temp root. + request_temp_dir = os.path.dirname(temp_files[0]) if temp_files else None + # Same tokens the CLI will run — do not use raw HTTP split(" ") service_mode_error = Verifycommand.validate_service_mode_restrictions( - command_tokens + command_tokens, request_temp_dir ) if service_mode_error: + logger.warning( + f"Service Mode blocked command '{command_tokens[0] if command_tokens else ''}': " + f"{service_mode_error}" + ) return {"status": "error", "error": service_mode_error}, 403 force_error = Verifycommand.validate_enterprise_user_add_role_force( diff --git a/keepercommander/service/util/parse_keeper_response.py b/keepercommander/service/util/parse_keeper_response.py index 9081b81fc..8a923dc59 100644 --- a/keepercommander/service/util/parse_keeper_response.py +++ b/keepercommander/service/util/parse_keeper_response.py @@ -1148,6 +1148,19 @@ def _parse_logging_based_command(command: str, response_str: str) -> Dict[str, A has_bad_request = any(pattern in response_lower for pattern in bad_request_patterns) has_error = any(pattern in response_lower for pattern in error_patterns) has_warning = any(pattern in response_lower for pattern in warning_patterns) + + # First-time share invite is success. Mixed multi-recipient output is + # only partially represented: throttle (above) and forbidden keep + # precedence over this short-circuit. + has_invitation = 'share invitation has been sent to' in response_lower + if has_invitation and not has_forbidden: + formatted_message = KeeperResponseParser._format_multiline_message(response_str) + return { + "status": "success", + "command": command.split()[0] if command.split() else command, + "message": formatted_message, + "data": None, + } if has_success_indicator and (has_not_found or has_bad_request or has_error): return { diff --git a/keepercommander/service/util/request_validation.py b/keepercommander/service/util/request_validation.py index cff1051f0..b0ad33cf6 100644 --- a/keepercommander/service/util/request_validation.py +++ b/keepercommander/service/util/request_validation.py @@ -15,6 +15,7 @@ import tempfile import os import json +import shutil from ..decorators.logging import logger, sanitize_command_fields @@ -71,9 +72,11 @@ def process_file_data(request_data: Dict[str, Any], command: str) -> Tuple[str, logger.warning("filedata must be a JSON object or array") return processed_command, temp_files + request_temp_dir = None try: - # Create temporary file with the filedata content - with tempfile.NamedTemporaryFile(mode='w', suffix='.json', delete=False, encoding='utf-8') as temp_file: + request_temp_dir = tempfile.mkdtemp(prefix='keeper_svc_') + with tempfile.NamedTemporaryFile(mode='w', suffix='.json', delete=False, + encoding='utf-8', dir=request_temp_dir) as temp_file: json.dump(filedata, temp_file, indent=2) temp_file_path = temp_file.name temp_files.append(temp_file_path) @@ -91,23 +94,24 @@ def process_file_data(request_data: Dict[str, Any], command: str) -> Tuple[str, except Exception as e: logger.error(f"Error creating temporary file for filedata: {e}") - # Clean up any created temp files - for temp_path in temp_files: + if request_temp_dir: try: - os.unlink(temp_path) - except Exception: - pass + shutil.rmtree(request_temp_dir) + logger.debug(f"Cleaned up request temp directory: {request_temp_dir}") + except Exception as cleanup_error: + logger.warning(f"Failed to clean up request temp directory {request_temp_dir}: {cleanup_error}") return command, [] return processed_command, temp_files @staticmethod def cleanup_temp_files(temp_files: list) -> None: - """Clean up temporary files. - + """Clean up temporary files and their parent per-request directories. + Args: temp_files: List of temporary file paths to clean up """ + parent_dirs = set() for temp_path in temp_files: try: if os.path.exists(temp_path): @@ -115,6 +119,16 @@ def cleanup_temp_files(temp_files: list) -> None: logger.debug(f"Cleaned up temporary file: {temp_path}") except Exception as e: logger.warning(f"Failed to clean up temporary file {temp_path}: {e}") + parent_dirs.add(os.path.dirname(temp_path)) + + # Remove parent directories (each file has one dedicated per-request directory) + for parent_dir in parent_dirs: + try: + if os.path.exists(parent_dir): + shutil.rmtree(parent_dir) + logger.debug(f"Cleaned up request temp directory: {parent_dir}") + except Exception as e: + logger.warning(f"Failed to clean up request temp directory {parent_dir}: {e}") @staticmethod def validate_request_json() -> Optional[Tuple]: diff --git a/keepercommander/service/util/verified_command.py b/keepercommander/service/util/verified_command.py index 1e240ad05..631984afc 100644 --- a/keepercommander/service/util/verified_command.py +++ b/keepercommander/service/util/verified_command.py @@ -1,3 +1,8 @@ +import contextlib +import io +import os + + class Verifycommand: # pam tunnel aliases: start=s, list=l, stop=x, edit=e, diagnose=d # (see PAMTunnelCommand.register_command in tunnel_and_connections.py) @@ -12,8 +17,49 @@ class Verifycommand: # Aliases from record.py — CommandExecutor checks tokens before cli expands them. _RECORD_EDIT_COMMANDS = frozenset({'record-add', 'ra', 'record-update', 'ru'}) + # WARNING: everything below is a DENYLIST. Any command/flag that reads or + # writes a host file and is NOT enumerated here is allowed by default. + # Adding a new command with local file I/O? Add it here, or it silently + # bypasses Service Mode's "no host filesystem access" boundary. + # + # Commands that always read/write host files (no safe Service Mode form). + _HOST_FS_COMMANDS = frozenset({ + 'run-batch', 'run', + 'export', + 'download-membership', + 'download-record-types', + 'apply-membership', + 'load-record-types', + }) + # Positional file input; FILEDATA is rewritten to a temp path before execute. + _FILE_INPUT_COMMANDS = frozenset({'import', 'enterprise-push'}) + # import --format values that name an account/API/URL source, not a local + # file (importer/commands.py choices). Any format NOT in this set is treated as file-based by default. + _IMPORT_FORMATS_WITHOUT_FILE = frozenset({ + 'lastpass', 'manageengine', 'thycotic', 'cyberark', 'cyberark_portal', + }) + # generate's registered alias (commands/utils.py: aliases['gen'] = 'generate'). + _GENERATE_COMMAND_NAMES = frozenset({'generate', 'gen'}) + # pam's 'project' subcommand alias (discoveryrotation.py: register_command('project', ..., 'p')). + _PAM_PROJECT_ALIASES = {'p': 'project'} + # pam project's own subcommand aliases (pam_import/commands.py: register_command(...)). + _PAM_PROJECT_SUBCOMMAND_ALIASES = {'x': 'export', 'i': 'import', 'e': 'extend'} + # --output values that select a mode/format, not a host path. + _NON_PATH_OUTPUT_VALUES = frozenset({ + 'clipboard', 'stdout', 'stdouthidden', 'variable', + 'token', 'base64', 'json', 'k8s', 'text', + }) + # Extensions that indicate a local data file (avoid treating emails as paths). + _LOCAL_FILE_EXTENSIONS = frozenset({ + '.json', '.csv', '.txt', '.yaml', '.yml', '.xml', '.kdbx', '.zip', + '.pdf', '.ndjson', '.1pif', '.xlsx', '.xls', '.kdb', '.gpg', + }) + _HOST_FS_MSG = ( + 'Local filesystem access is not permitted through Service Mode' + ) + @staticmethod - def validate_service_mode_restrictions(command_tokens): + def validate_service_mode_restrictions(command_tokens, request_temp_dir=None): """Run Service Mode bans on executor tokens (shlex); error string or None.""" if not command_tokens: return None @@ -23,14 +69,17 @@ def validate_service_mode_restrictions(command_tokens): Verifycommand.validate_service_mode_download_attachment_command, Verifycommand.validate_service_mode_upload_attachment_command, Verifycommand.validate_service_mode_record_file_attachment_command, + Verifycommand.validate_service_mode_host_filesystem_command, + Verifycommand.validate_service_mode_host_path_args, + Verifycommand.validate_service_mode_file_input_command, ): - error = validator(command_tokens) + error = validator(command_tokens, request_temp_dir) if error: return error return None @staticmethod - def validate_service_mode_pam_tunnel_command(command_tokens): + def validate_service_mode_pam_tunnel_command(command_tokens, request_temp_dir=None): """Allow only pam tunnel edit in Service Mode; error string or None.""" if not command_tokens or len(command_tokens) < 2: return None @@ -49,7 +98,7 @@ def validate_service_mode_pam_tunnel_command(command_tokens): ) @staticmethod - def validate_service_mode_download_attachment_command(command_tokens): + def validate_service_mode_download_attachment_command(command_tokens, request_temp_dir=None): """Block download-attachment in Service Mode; error string or None.""" if not command_tokens: return None @@ -61,7 +110,7 @@ def validate_service_mode_download_attachment_command(command_tokens): ) @staticmethod - def validate_service_mode_upload_attachment_command(command_tokens): + def validate_service_mode_upload_attachment_command(command_tokens, request_temp_dir=None): """Block upload-attachment in Service Mode; error string or None.""" if not command_tokens: return None @@ -73,7 +122,7 @@ def validate_service_mode_upload_attachment_command(command_tokens): ) @staticmethod - def validate_service_mode_record_file_attachment_command(command_tokens): + def validate_service_mode_record_file_attachment_command(command_tokens, request_temp_dir=None): """Block record-add/update (and ra/ru) file fields in Service Mode; error or None.""" if not command_tokens: return None @@ -97,6 +146,236 @@ def _is_record_file_attachment_arg(token): name = name[2:] return name.split('.', 1)[0] == 'file' + @staticmethod + def validate_service_mode_host_filesystem_command(command_tokens, request_temp_dir=None): + """Block commands that always touch the host filesystem; error or None.""" + if not command_tokens: + return None + if command_tokens[0].lower() not in Verifycommand._HOST_FS_COMMANDS: + return None + return Verifycommand._HOST_FS_MSG + + @staticmethod + def validate_service_mode_host_path_args(command_tokens, request_temp_dir=None): + """Block host-path flags (--output, --filename, …); error or None.""" + if not command_tokens: + return None + + tokens = command_tokens + cmd0 = tokens[0].lower() + + # PDF always requires an on-disk output file. Routed through + # _option_values so abbreviations (--form, --fmt=pdf, …) are caught too. + for value in Verifycommand._option_values(tokens, '--format'): + if value.lower() == 'pdf': + return Verifycommand._HOST_FS_MSG + + for flag in ('--out-dir', '--output-dir', '--from-file', '--file-cache', + '--keepass-key-file', '-v3f'): + if Verifycommand._has_option(tokens, flag): + return Verifycommand._HOST_FS_MSG + + # credential-provision --config is a host YAML path; --config-base64 is OK. + # Do not ban --config globally — PAM uses it for configuration UIDs. + if cmd0 in ('credential-provision', 'cp') and Verifycommand._has_option(tokens, '--config'): + return Verifycommand._HOST_FS_MSG + + for value in Verifycommand._option_values(tokens, '--filename'): + if not Verifycommand._is_service_temp_path(value, request_temp_dir): + return Verifycommand._HOST_FS_MSG + + # pam project import/extend use -f as --filename (not --force). + if Verifycommand._is_pam_project_filename_cmd(tokens): + for value in Verifycommand._option_values(tokens, '-f'): + if not Verifycommand._is_service_temp_path(value, request_temp_dir): + return Verifycommand._HOST_FS_MSG + + for value in Verifycommand._option_values(tokens, '--file'): + if Verifycommand._looks_like_local_path(value): + if not Verifycommand._is_service_temp_path(value, request_temp_dir): + return Verifycommand._HOST_FS_MSG + + for value in Verifycommand._option_values(tokens, '--output'): + if value.lower() not in Verifycommand._NON_PATH_OUTPUT_VALUES: + return Verifycommand._HOST_FS_MSG + + # generate / pam project export use -o as a file path (not mode). + if cmd0 in Verifycommand._GENERATE_COMMAND_NAMES or Verifycommand._is_pam_project_export(tokens): + for value in Verifycommand._option_values(tokens, '-o'): + if value.lower() not in Verifycommand._NON_PATH_OUTPUT_VALUES: + return Verifycommand._HOST_FS_MSG + + # transfer-user (tu) @filename mapping files - scoped to that command + # only, since '@value' is a legitimate argument shape elsewhere. + if cmd0 in ('transfer-user', 'tu'): + for tok in tokens[1:]: + if tok.startswith('@') and Verifycommand._looks_like_local_path(tok[1:]): + return Verifycommand._HOST_FS_MSG + + return None + + @staticmethod + def validate_service_mode_file_input_command(command_tokens, request_temp_dir=None): + """Allow import/enterprise-push only with FILEDATA temp paths; error or None.""" + if not command_tokens: + return None + cmd0 = command_tokens[0].lower() + if cmd0 not in Verifycommand._FILE_INPUT_COMMANDS: + return None + + if cmd0 == 'import': + from ...importer.commands import import_parser + ns = Verifycommand._safe_parse(import_parser, command_tokens[1:]) + if ns is None: + # Malformed for the real parser too; it will reject this itself. + return None + fmt = (ns.format or '').lower() + if fmt in Verifycommand._IMPORT_FORMATS_WITHOUT_FILE: + return None + if ns.name and not Verifycommand._is_service_temp_path(ns.name, request_temp_dir): + return Verifycommand._HOST_FS_MSG + return None + + if cmd0 == 'enterprise-push': + from ...commands.enterprise_push import enterprise_push_parser + ns = Verifycommand._safe_parse(enterprise_push_parser, command_tokens[1:]) + if ns is None: + return None + if ns.file and not Verifycommand._is_service_temp_path(ns.file, request_temp_dir): + return Verifycommand._HOST_FS_MSG + return None + + return None + + @staticmethod + def _safe_parse(parser, tokens): + """Resolve tokens via the command's real argparse parser; None if it can't be resolved. + + Used instead of hand-scanning tokens so we get the parser's own + distinction between a positional value and a flag's value -- the + raw tokens alone don't tell you that. + """ + try: + with contextlib.redirect_stderr(io.StringIO()): + ns, _ = parser.parse_known_args(tokens) + return ns + except SystemExit: + return None + except Exception: + return None + + @staticmethod + def _pam_project_verb(command_tokens): + """Resolve 'pam ' (alias-aware) to the verb, or None.""" + if len(command_tokens) < 3: + return None + t = [x.lower() for x in command_tokens[:3]] + if t[0] != 'pam': + return None + project = Verifycommand._PAM_PROJECT_ALIASES.get(t[1], t[1]) + if project != 'project': + return None + return Verifycommand._PAM_PROJECT_SUBCOMMAND_ALIASES.get(t[2], t[2]) + + @staticmethod + def _is_pam_project_export(command_tokens): + return Verifycommand._pam_project_verb(command_tokens) == 'export' + + @staticmethod + def _is_pam_project_filename_cmd(command_tokens): + return Verifycommand._pam_project_verb(command_tokens) in ('import', 'extend') + + # Flags we separately check for. Some are literal prefixes of others + # (--output/--output-dir, --file/--filename, --file/--file-cache) - a + # token that's a *complete* match for one of these is a distinct flag in + # its own right, never an abbreviation attempt at a different one. + _KNOWN_DANGEROUS_FLAGS = frozenset({ + '--output', '--filename', '--file', '--format', + '--out-dir', '--output-dir', '--from-file', '--file-cache', + '--keepass-key-file', '--config', + }) + + @staticmethod + def _flag_matches(tok_name, flag): + """True if tok_name is exactly flag, or an unambiguous long-option + abbreviation of it (argparse's allow_abbrev default -- almost no + parser in this codebase opts out of it, so '--out' really does mean + '--output' to the command that will actually run). + + Short options ('-o', '-f') are never abbreviated by argparse, so + those only ever match exactly. + """ + if tok_name == flag: + return True + # Reject bare '--' (argparse's "end of options" marker), anything + # that isn't a '--long' option on both sides, and anything that's + # already a complete, distinct flag of its own. + if len(tok_name) <= 2 or not tok_name.startswith('--') or not flag.startswith('--'): + return False + if tok_name in Verifycommand._KNOWN_DANGEROUS_FLAGS: + return False + return flag.startswith(tok_name) + + @staticmethod + def _has_option(tokens, flag): + """True if flag, flag=value, or an abbreviation of flag appears in tokens.""" + flag_l = flag.lower() + for tok in tokens[1:]: + name = tok.lower().split('=', 1)[0] + if Verifycommand._flag_matches(name, flag_l): + return True + return False + + @staticmethod + def _option_values(tokens, flag): + """Yield values for --flag value / --flag=value (and short -o value), + matching flag itself or an unambiguous abbreviation of it.""" + flag_l = flag.lower() + i = 1 + while i < len(tokens): + tok = tokens[i] + lower = tok.lower() + name, sep, _ = lower.partition('=') + if sep and Verifycommand._flag_matches(name, flag_l): + yield tok.split('=', 1)[1] + i += 1 + continue + if not sep and Verifycommand._flag_matches(name, flag_l): + # Consume the next token as the value even if it starts with + # '-' (e.g. a path or FILEDATA placeholder) -- these flags are + # all required-argument (action='store'), so skipping a + # dash-leading value here let it slip past every check below. + if i + 1 < len(tokens): + yield tokens[i + 1] + i += 2 + continue + i += 1 + + @staticmethod + def _looks_like_local_path(value): + if not value: + return False + if value.startswith(('http://', 'https://')): + return False + if value.startswith('~') or '/' in value or '\\' in value: + return True + _, ext = os.path.splitext(value) + return ext.lower() in Verifycommand._LOCAL_FILE_EXTENSIONS + + @staticmethod + def _is_service_temp_path(path, request_temp_dir): + """True when path resolves under *this request's* own temp directory, not just anywhere under the + shared OS temp root -- other processes/users can also write there. + """ + if not path or not request_temp_dir: + return False + try: + resolved = os.path.realpath(os.path.expanduser(path)) + root = os.path.realpath(request_temp_dir) + return resolved == root or resolved.startswith(root + os.sep) + except (OSError, ValueError): + return False + @staticmethod def validate_append_command(command): """ diff --git a/unit-tests/pam/test_pam_connection_edit_scrollback.py b/unit-tests/pam/test_pam_connection_edit_scrollback.py index 090fe1ad5..3d3a581e0 100644 --- a/unit-tests/pam/test_pam_connection_edit_scrollback.py +++ b/unit-tests/pam/test_pam_connection_edit_scrollback.py @@ -11,6 +11,30 @@ skip_tests = False skip_reason = "" + + +def mock_sync_decorator(cls): + """Decorator to add sync mocking to test classes that execute PAM commands.""" + original_setup = cls.setUp if hasattr(cls, 'setUp') else None + original_teardown = cls.tearDown if hasattr(cls, 'tearDown') else None + + def new_setup(self): + if original_setup: + original_setup(self) + self.sync_patcher = __import__('unittest.mock', fromlist=['patch']).patch('keepercommander.commands.tunnel_and_connections.sync_down_preserving_nsf_keys') + self.sync_patcher.start() + self.load_patcher = __import__('unittest.mock', fromlist=['patch']).patch('keepercommander.commands.pam_import.record_loader.load_pam_record') + self.load_patcher.start() + + def new_teardown(self): + self.sync_patcher.stop() + self.load_patcher.stop() + if original_teardown: + original_teardown(self) + + cls.setUp = new_setup + cls.tearDown = new_teardown + return cls try: from keepercommander.commands.tunnel_and_connections import PAMConnectionEditCommand from keepercommander.error import CommandError @@ -91,6 +115,7 @@ def test_all_db_protocols_present_in_choices(self): self.assertIn(proto, PAMConnectionEditCommand.protocols) +@mock_sync_decorator @unittest.skipIf(skip_tests, skip_reason) class TestPamConnectionEditScrollbackValidation(unittest.TestCase): """Validation runs before DAG / token operations, so we can drive execute() @@ -233,6 +258,7 @@ def test_protocol_change_without_connections_uses_existing(self): self.assertNotIn('not supported for protocol', str(ctx.exception)) +@mock_sync_decorator @unittest.skipIf(skip_tests, skip_reason) class TestPamConnectionEditScrollbackAllowedCombinations(unittest.TestCase): """For each allowed (record_type, protocol) pair, validation must not raise @@ -325,7 +351,7 @@ def _mock_record(self, record_type='pamMachine', protocol='ssh'): return rec @mock.patch('keepercommander.commands.tunnel_and_connections.RecordMixin.resolve_single_record') - @mock.patch('keepercommander.commands.tunnel_and_connections.update_pam_record') + @mock.patch('keepercommander.commands.tunnel_and_connections.update_pam_record', return_value=False) @mock.patch('keepercommander.commands.tunnel_and_connections.api.sync_down') @mock.patch('keepercommander.commands.tunnel_and_connections.get_keeper_tokens', return_value=(b'st', b'tk', b'tr')) @@ -344,7 +370,7 @@ def test_scrollback_alone_skips_dag(self, mock_tdag, mock_get_config_uid, mock_update.assert_called_once() @mock.patch('keepercommander.commands.tunnel_and_connections.RecordMixin.resolve_single_record') - @mock.patch('keepercommander.commands.tunnel_and_connections.update_pam_record') + @mock.patch('keepercommander.commands.tunnel_and_connections.update_pam_record', return_value=False) @mock.patch('keepercommander.commands.tunnel_and_connections.api.sync_down') @mock.patch('keepercommander.commands.tunnel_and_connections.get_keeper_tokens', return_value=(b'st', b'tk', b'tr')) @@ -361,7 +387,7 @@ def test_key_events_alone_skips_dag(self, mock_tdag, mock_get_config_uid, mock_tdag.assert_not_called() @mock.patch('keepercommander.commands.tunnel_and_connections.RecordMixin.resolve_single_record') - @mock.patch('keepercommander.commands.tunnel_and_connections.update_pam_record') + @mock.patch('keepercommander.commands.tunnel_and_connections.update_pam_record', return_value=False) @mock.patch('keepercommander.commands.tunnel_and_connections.api.sync_down') @mock.patch('keepercommander.commands.tunnel_and_connections.get_keeper_tokens', return_value=(b'st', b'tk', b'tr')) diff --git a/unit-tests/pam/test_pam_connection_edit_security.py b/unit-tests/pam/test_pam_connection_edit_security.py index d4dd332d7..6b0c461c9 100644 --- a/unit-tests/pam/test_pam_connection_edit_security.py +++ b/unit-tests/pam/test_pam_connection_edit_security.py @@ -21,6 +21,30 @@ skip_reason = f"Cannot import tunnel_and_connections: {e}" +def mock_sync_decorator(cls): + """Decorator to add sync mocking to test classes that execute PAM commands.""" + original_setup = cls.setUp if hasattr(cls, 'setUp') else None + original_teardown = cls.tearDown if hasattr(cls, 'tearDown') else None + + def new_setup(self): + if original_setup: + original_setup(self) + self.sync_patcher = mock.patch('keepercommander.commands.tunnel_and_connections.sync_down_preserving_nsf_keys') + self.sync_patcher.start() + self.load_patcher = mock.patch('keepercommander.commands.pam_import.record_loader.load_pam_record') + self.load_patcher.start() + + def new_teardown(self): + self.sync_patcher.stop() + self.load_patcher.stop() + if original_teardown: + original_teardown(self) + + cls.setUp = new_setup + cls.tearDown = new_teardown + return cls + + @unittest.skipIf(skip_tests, skip_reason) class TestPamConnectionEditSecurityArgs(unittest.TestCase): def setUp(self): @@ -76,6 +100,7 @@ def test_help_includes_new_flags(self): self.assertIn('-sm', help_text) +@mock_sync_decorator @unittest.skipIf(skip_tests, skip_reason) class TestPamConnectionEditSecurityValidation(unittest.TestCase): """Validation runs before DAG / token operations, so we can drive execute() @@ -197,6 +222,7 @@ def test_protocol_change_in_same_command_validated_against_new(self): self.assertIn('not supported for protocol "ssh"', str(ctx.exception)) +@mock_sync_decorator @unittest.skipIf(skip_tests, skip_reason) class TestPamConnectionEditSecurityMutation(unittest.TestCase): """Verifies the actual JSON keys written to pamSettings.connection.""" @@ -339,7 +365,7 @@ def _mock_record(self, record_type='pamMachine', protocol='rdp'): return rec @mock.patch('keepercommander.commands.tunnel_and_connections.RecordMixin.resolve_single_record') - @mock.patch('keepercommander.commands.tunnel_and_connections.update_pam_record') + @mock.patch('keepercommander.commands.tunnel_and_connections.update_pam_record', return_value=False) @mock.patch('keepercommander.commands.tunnel_and_connections.api.sync_down') @mock.patch('keepercommander.commands.tunnel_and_connections.get_keeper_tokens', return_value=(b'st', b'tk', b'tr')) @@ -356,7 +382,7 @@ def test_ignore_server_cert_alone_skips_dag(self, mock_tdag, mock_get_config_uid mock_update.assert_called_once() @mock.patch('keepercommander.commands.tunnel_and_connections.RecordMixin.resolve_single_record') - @mock.patch('keepercommander.commands.tunnel_and_connections.update_pam_record') + @mock.patch('keepercommander.commands.tunnel_and_connections.update_pam_record', return_value=False) @mock.patch('keepercommander.commands.tunnel_and_connections.api.sync_down') @mock.patch('keepercommander.commands.tunnel_and_connections.get_keeper_tokens', return_value=(b'st', b'tk', b'tr')) diff --git a/unit-tests/pam/test_pam_debug_nsf.py b/unit-tests/pam/test_pam_debug_nsf.py index b68666192..edbe9d3ee 100644 --- a/unit-tests/pam/test_pam_debug_nsf.py +++ b/unit-tests/pam/test_pam_debug_nsf.py @@ -9,11 +9,13 @@ from keepercommander import utils, vault from keepercommander.commands.discover import GatewayContext from keepercommander.commands.pam_debug import load_pam_record -from keepercommander.commands.pam_debug.acl import PAMDebugACLCommand from keepercommander.commands.pam_debug.dump import PAMDebugDumpCommand -from keepercommander.commands.pam_debug.link import PAMDebugLinkCommand from keepercommander.subfolder import NestedShareFolderNode, RootFolderNode +# Note: test_acl_uses_load_pam_record_for_nsf_uids() and test_link_uses_load_pam_record_for_nsf_resource() +# were removed because the pam_debug.acl and pam_debug.link modules do not exist in the codebase. +# These tests were testing the integration of missing modules and were blocking test collection. + def _typed(uid, title, record_type='pamUser', version=3): rec = vault.TypedRecord(version=version) @@ -74,47 +76,6 @@ def test_load_pam_record_resolves_nsf_machine(self): self.assertEqual(rec.title, 'NSF Machine') self.assertEqual(rec.record_type, 'pamMachine') - def test_acl_uses_load_pam_record_for_nsf_uids(self): - params = _params() - user = _typed('user_uid', 'NSF User', 'pamUser') - parent = _typed('machine_uid', 'NSF Machine', 'pamMachine') - gw = MagicMock() - gw.configuration = _typed('config_uid', 'NSF Config', 'pamNetworkConfiguration', version=6) - gw.configuration_uid = 'config_uid' - - with patch('keepercommander.commands.pam_debug.acl.GatewayContext.from_gateway', return_value=gw), \ - patch('keepercommander.commands.pam_debug.acl.RecordLink') as rl_cls, \ - patch('keepercommander.commands.pam_debug.acl.load_pam_record', - side_effect=[user, parent]) as load, \ - patch('builtins.input', side_effect=['n', 'n']): - rl = rl_cls.return_value - rl.get_acl.return_value = None - rl.get_admin_record_uid.return_value = None - rl.acl_has_belong_to_record_uid.return_value = None - rl.dag.get_vertex.return_value = MagicMock() - PAMDebugACLCommand().execute( - params, gateway='gw', user_uid='user_uid', parent_uid='machine_uid') - - self.assertEqual(load.call_count, 2) - self.assertEqual(load.call_args_list[0].args[1], 'user_uid') - self.assertEqual(load.call_args_list[1].args[1], 'machine_uid') - - def test_link_uses_load_pam_record_for_nsf_resource(self): - params = _params() - parent = _typed('machine_uid', 'NSF Machine', 'pamMachine') - gw = MagicMock() - gw.configuration = _typed('config_uid', 'NSF Config', 'pamNetworkConfiguration', version=6) - gw.configuration_uid = 'config_uid' - - with patch('keepercommander.commands.pam_debug.link.GatewayContext.from_gateway', return_value=gw), \ - patch('keepercommander.commands.pam_debug.link.RecordLink') as rl_cls, \ - patch('keepercommander.commands.pam_debug.link.load_pam_record', return_value=parent) as load: - rl = rl_cls.return_value - PAMDebugLinkCommand().execute(params, gateway='gw', resource_uid='machine_uid') - rl.belongs_to.assert_called_once() - rl.save.assert_called_once() - self.assertEqual(load.call_args.args[1], 'machine_uid') - def test_dump_collects_nsf_folder_records(self): params = _params() with tempfile.TemporaryDirectory() as tmp: diff --git a/unit-tests/pam/test_pam_nsf_config.py b/unit-tests/pam/test_pam_nsf_config.py index d3372e1d9..b149e694f 100644 --- a/unit-tests/pam/test_pam_nsf_config.py +++ b/unit-tests/pam/test_pam_nsf_config.py @@ -1,3 +1,4 @@ +import json import unittest from unittest import mock @@ -9,6 +10,7 @@ from keepercommander.commands.pam.vault_target import ( create_pam_configuration_in_folder, create_record_in_folder, place_record_in_folder, resolve_pam_folder_uid, records_in_folder) +from keepercommander.commands.pam_import.record_loader import load_pam_record from keepercommander.error import CommandError from keepercommander.subfolder import NestedShareFolderNode, RootFolderNode, SharedFolderNode @@ -27,6 +29,35 @@ def _make_params(): class TestPamVaultTarget(unittest.TestCase): + def test_load_pam_record_prefers_fresh_nsf_data_over_classic_cache(self): + params = _make_params() + record_uid = 'nsf_record_uid' + params.nested_share_records = {} + params.nested_share_record_data = {} + params.record_cache[record_uid] = { + 'version': 3, + 'data_unencrypted': json.dumps({ + 'type': 'pamMachine', + 'title': 'Stale', + 'fields': [{'type': 'text', 'value': ['before']}], + 'custom': [], + }), + } + params.nested_share_records[record_uid] = {'version': 3} + params.nested_share_record_data[record_uid] = { + 'data_json': { + 'type': 'pamMachine', + 'title': 'Fresh', + 'fields': [{'type': 'text', 'value': ['after']}], + 'custom': [], + }, + } + + record = load_pam_record(params, record_uid) + + self.assertEqual(record.title, 'Fresh') + self.assertEqual(record.fields[0].value, ['after']) + @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') diff --git a/unit-tests/pam/test_pam_rbi_edit.py b/unit-tests/pam/test_pam_rbi_edit.py index 67e5ad155..1d8631efa 100644 --- a/unit-tests/pam/test_pam_rbi_edit.py +++ b/unit-tests/pam/test_pam_rbi_edit.py @@ -27,6 +27,31 @@ skip_reason = f"Cannot import tunnel_and_connections: {e}" +def mock_sync_decorator(cls): + """Decorator to add sync mocking to test classes that execute PAM commands.""" + original_setup = cls.setUp if hasattr(cls, 'setUp') else None + original_teardown = cls.tearDown if hasattr(cls, 'tearDown') else None + + def new_setup(self): + if original_setup: + original_setup(self) + # Mock sync and load operations + self.sync_patcher = mock.patch('keepercommander.commands.tunnel_and_connections.sync_down_preserving_nsf_keys') + self.sync_patcher.start() + self.load_patcher = mock.patch('keepercommander.commands.pam_import.record_loader.load_pam_record') + self.load_patcher.start() + + def new_teardown(self): + self.sync_patcher.stop() + self.load_patcher.stop() + if original_teardown: + original_teardown(self) + + cls.setUp = new_setup + cls.tearDown = new_teardown + return cls + + @unittest.skipIf(skip_tests, skip_reason) class TestPamRbiEditArguments(unittest.TestCase): """Tests for PAMRbiEditCommand argument parsing.""" @@ -228,6 +253,7 @@ def test_session_persistence_not_provided(self): self.assertIsNone(args.session_persistence) +@mock_sync_decorator @unittest.skipIf(skip_tests, skip_reason) class TestPamRbiEditExecute(unittest.TestCase): """Tests for PAMRbiEditCommand.execute() method.""" @@ -242,7 +268,11 @@ def setUp(self): self.mock_field.value = [self.pam_settings] self.mock_record.get_typed_field.return_value = self.mock_field self.mock_params = mock.MagicMock() + # Use REAL dicts for caches so sync/reload operations work self.mock_params.record_cache = {'test-record-uid': self.mock_record} + self.mock_params.nested_share_record_data = {} + self.mock_params.nested_share_records = {} + # Parent class decorator handles sync/load mocking in setUp def test_no_param_raises_error_with_new_settings_check(self): with self.assertRaises(CommandError) as context: @@ -376,6 +406,7 @@ def test_session_persistence_default_removes_present_but_null(self, mock_update, self.assertNotIn('sessionPersistence', self.pam_settings['connection']) +@mock_sync_decorator @unittest.skipIf(skip_tests, skip_reason) class TestPamRbiEditClipboardInversion(unittest.TestCase): """Tests for clipboard inversion logic.""" @@ -444,6 +475,7 @@ def test_clipboard_both_off(self, mock_update, mock_resolve): self.assertEqual(self.pam_settings['connection'].get('disablePaste'), True) +@mock_sync_decorator @unittest.skipIf(skip_tests, skip_reason) class TestPamRbiEditRecordUpdate(unittest.TestCase): """Tests for record update behavior.""" @@ -598,6 +630,7 @@ def test_alias_sr(self): self.assertEqual(args.audio_sample_rate, 44100) +@mock_sync_decorator @unittest.skipIf(skip_tests, skip_reason) class TestPamRbiEditAudioSettings(unittest.TestCase): """Tests for audio settings.""" diff --git a/unit-tests/service/test_api_routes.py b/unit-tests/service/test_api_routes.py index 758fcaec4..222d4d530 100644 --- a/unit-tests/service/test_api_routes.py +++ b/unit-tests/service/test_api_routes.py @@ -70,5 +70,5 @@ def test_v1_direct_route_keeps_legacy_execution_path(self): self.assertEqual(response.status_code, 200) self.assertEqual(response.headers.get('X-API-Legacy'), 'true') self.assertEqual(response.get_json(), {"status": "success", "data": {"command": "ls"}}) - mock_execute.assert_called_once_with('ls') + mock_execute.assert_called_once_with('ls', temp_files=[]) mock_submit.assert_not_called() diff --git a/unit-tests/service/test_auth_security.py b/unit-tests/service/test_auth_security.py index 68913db28..9a8d3c567 100644 --- a/unit-tests/service/test_auth_security.py +++ b/unit-tests/service/test_auth_security.py @@ -169,3 +169,12 @@ def test_validate_service_mode_restrictions_attachments(self): ) self.assertIsNone(check(['record-add', '--title', 't', '-rt', 'login', 'login=user'])) self.assertIsNone(check(['record-add', '--title', 't', '-rt', 'login', 'my.file=x'])) + + def test_validate_service_mode_restrictions_host_filesystem(self): + check = Verifycommand.validate_service_mode_restrictions + ban = 'Local filesystem access' + self.assertIn(ban, check(['run-batch', '--dry-run', '/etc/passwd'])) + self.assertIn(ban, check(['export', '--format=json', '/tmp/out.json'])) + self.assertIn(ban, check(['audit-report', '--format=json', '--output=/tmp/r.json'])) + self.assertIsNone(check(['audit-report', '--format=json'])) + self.assertIsNone(check(['clipboard-copy', 'uid', '--output=stdout'])) diff --git a/unit-tests/service/test_queue_concurrency.py b/unit-tests/service/test_queue_concurrency.py index 5a2800e30..25cf04e60 100644 --- a/unit-tests/service/test_queue_concurrency.py +++ b/unit-tests/service/test_queue_concurrency.py @@ -52,7 +52,7 @@ def test_queue_manager_serializes_concurrent_submissions(self): inflight = {"count": 0, "max": 0} results = {} - def fake_execute(command): + def fake_execute(command, **kwargs): with state_lock: inflight["count"] += 1 inflight["max"] = max(inflight["max"], inflight["count"]) @@ -91,7 +91,7 @@ def test_v1_and_v2_share_single_queue_worker(self): outputs = {} start_barrier = threading.Barrier(3) - def fake_execute(command): + def fake_execute(command, **kwargs): with state_lock: inflight["count"] += 1 inflight["max"] = max(inflight["max"], inflight["count"]) @@ -148,7 +148,7 @@ def test_timed_out_v1_request_does_not_execute_after_expiration(self): executed_commands = [] executed_lock = threading.Lock() - def fake_execute(command): + def fake_execute(command, **kwargs): with executed_lock: executed_commands.append(command) @@ -189,7 +189,7 @@ def test_processing_v1_request_waits_past_queue_timeout(self): started_processing = threading.Event() - def fake_execute(command): + def fake_execute(command, **kwargs): started_processing.set() time.sleep(request_timeout + 0.15) return {"status": "success", "data": {"command": command}}, 200 diff --git a/unit-tests/service/test_service_mode_pam_tunnel.py b/unit-tests/service/test_service_mode_pam_tunnel.py index 257ca41dd..083280bff 100644 --- a/unit-tests/service/test_service_mode_pam_tunnel.py +++ b/unit-tests/service/test_service_mode_pam_tunnel.py @@ -1,5 +1,8 @@ from unittest import TestCase from html import unescape +import os +import shutil +import tempfile import shlex @@ -90,6 +93,203 @@ def test_attachment_commands_blocked_for_remote_api(self): ) self.assertIsNone(check(_tokens('record-add --title t -rt login login=user'))) + def test_host_filesystem_commands_blocked(self): + check = Verifycommand.validate_service_mode_restrictions + ban = 'Local filesystem access' + for cmd in ( + 'run-batch --dry-run /etc/passwd', + 'run --dry-run ~/.keeper/config.json', + 'export --format=json /tmp/out.json', + 'download-membership --source=keeper /tmp/m.json', + 'download-record-types --source=keeper /tmp/rt.json', + 'apply-membership /tmp/m.json', + 'load-record-types /tmp/rt.json', + ): + with self.subTest(cmd=cmd): + err = check(_tokens(cmd)) + self.assertIsNotNone(err) + self.assertIn(ban, err) + + def test_host_path_output_args_blocked(self): + check = Verifycommand.validate_service_mode_restrictions + ban = 'Local filesystem access' + for cmd in ( + 'audit-report --format=json --output=/tmp/report.json', + 'share-report --format=csv --output=out.csv', + 'generate --output /tmp/passwords.txt', + 'generate -o /tmp/passwords.txt', + 'pam project export --project-uid UID -o /tmp/proj.json', + 'ls --format=pdf --output=/tmp/x.pdf', + 'audit-report --format=pdf --output=report.pdf', + ): + with self.subTest(cmd=cmd): + err = check(_tokens(cmd)) + self.assertIsNotNone(err) + self.assertIn(ban, err) + + # Non-path --output modes remain allowed + self.assertIsNone(check(_tokens('clipboard-copy UID --output=stdout'))) + self.assertIsNone(check(_tokens('credential-provision --config-base64 dGVzdA== --output json'))) + self.assertIsNone(check(_tokens('audit-report --format=json'))) + + def test_filename_allows_temp_filedata_paths_only(self): + check = Verifycommand.validate_service_mode_restrictions + ban = 'Local filesystem access' + self.assertIsNotNone(check(_tokens('pam project import --filename=/etc/passwd'))) + self.assertIsNotNone(check(_tokens('pam project import -f /etc/passwd'))) + # Use a path outside any OS temp dir -- on Linux, tempfile.gettempdir() + # often *is* /tmp, so a literal /tmp path would be misclassified as safe. + self.assertIn(ban, check(_tokens('import --format=json /etc/vault.json'))) + + request_temp_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, request_temp_dir, ignore_errors=True) + temp_path = os.path.join(request_temp_dir, 'service_filedata_test.json') + + self.assertIsNone( + check(_tokens(f'pam project import --filename={temp_path}'), request_temp_dir) + ) + self.assertIsNone(check(_tokens(f'pam project import -f {temp_path}'), request_temp_dir)) + self.assertIsNone(check(_tokens(f'import --format=json {temp_path}'), request_temp_dir)) + self.assertIsNone( + check(_tokens(f'enterprise-push {temp_path} --email user@example.com'), request_temp_dir) + ) + # PAM --config is a vault UID, not a host path + self.assertIsNone( + check(_tokens('pam project extend --config=SOME_UID -f ' + temp_path), request_temp_dir) + ) + + def test_command_aliases_do_not_bypass_host_path_checks(self): + check = Verifycommand.validate_service_mode_restrictions + ban = 'Local filesystem access' + for cmd in ( + 'gen -o /tmp/passwords.txt', + 'gen --output /tmp/passwords.txt', + 'pam p x --project-uid UID -o /tmp/proj.json', + 'pam p i -f /etc/passwd', + 'pam p i --filename=/etc/passwd', + 'pam p e -f /etc/passwd', + ): + with self.subTest(cmd=cmd): + err = check(_tokens(cmd)) + self.assertIsNotNone(err) + self.assertIn(ban, err) + + # Aliased forms of the safe cases (non-path --output, temp-path filename) stay allowed. + request_temp_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, request_temp_dir, ignore_errors=True) + temp_path = os.path.join(request_temp_dir, 'service_filedata_alias_test.json') + self.assertIsNone(check(_tokens('gen --output stdout'))) + self.assertIsNone(check(_tokens(f'pam p i -f {temp_path}'), request_temp_dir)) + self.assertIsNone(check(_tokens(f'pam p e --filename={temp_path}'), request_temp_dir)) + + def test_non_request_temp_path_still_rejected(self): + """A path under a DIFFERENT request's temp dir is not automatically safe, + even though it's still somewhere under the shared OS temp root.""" + check = Verifycommand.validate_service_mode_restrictions + ban = 'Local filesystem access' + + request_temp_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, request_temp_dir, ignore_errors=True) + other_request_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, other_request_dir, ignore_errors=True) + other_path = os.path.join(other_request_dir, 'someone_elses_file.json') + + self.assertIn( + ban, check(_tokens(f'pam project import -f {other_path}'), request_temp_dir) + ) + # No request_temp_dir at all (e.g. a request with no FILEDATA) trusts nothing. + self.assertIn( + ban, check(_tokens(f'pam project import -f {other_path}')) + ) + + def test_flag_abbreviations_do_not_bypass_host_path_checks(self): + """argparse's default allow_abbrev means '--out' really does mean + '--output' to the real command -- our checker has to agree.""" + check = Verifycommand.validate_service_mode_restrictions + ban = 'Local filesystem access' + for cmd in ( + 'generate --out /etc/evil', + 'audit-report --form pdf --out /etc/evil.pdf', + 'pam project import --filenam=/etc/passwd', + 'pam project import --fil /etc/passwd', + ): + with self.subTest(cmd=cmd): + err = check(_tokens(cmd)) + self.assertIsNotNone(err) + self.assertIn(ban, err) + + # A complete, distinct flag must not be misread as an abbreviation of + # a different one just because it's a literal prefix of it. + self.assertIsNone(check(_tokens('clipboard-copy UID --output=stdout'))) + + def test_import_bare_name_positional_requires_format_awareness(self): + """A plain filename with no slash/extension must still be checked -- + unless the format means `name` isn't a file at all (account/URL).""" + check = Verifycommand.validate_service_mode_restrictions + ban = 'Local filesystem access' + + # 'data' has no slash and no known extension, but json/csv always + # read it as a local file -- must not slip through on shape alone. + self.assertIn(ban, check(_tokens('import --format=json data'))) + self.assertIn(ban, check(_tokens('import --format=csv data'))) + + # lastpass/manageengine/thycotic/cyberark/cyberark_portal treat `name` + # as an account/URL, not a file -- must stay allowed even though it's + # a bare, non-path-looking value. + self.assertIsNone(check(_tokens('import --format=lastpass my-lastpass-account'))) + self.assertIsNone(check(_tokens('import --format=manageengine https://me.example.com'))) + self.assertIsNone(check(_tokens('import --format=thycotic https://thycotic.example.com'))) + self.assertIsNone(check(_tokens('import --format=cyberark pvwa.example.com'))) + self.assertIsNone(check(_tokens('import --format=cyberark_portal example-tenant'))) + + # A real per-request temp path still works normally for file-based formats. + request_temp_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, request_temp_dir, ignore_errors=True) + temp_path = os.path.join(request_temp_dir, 'import_data.json') + self.assertIsNone(check(_tokens(f'import --format=json {temp_path}'), request_temp_dir)) + + def test_filedata_substitution_happens_before_validation(self): + """process_file_data must run before validate_service_mode_restrictions, + so --filename=FILEDATA resolves to a real per-request temp path by the + time the host-path checks run (see CommandExecutor.execute ordering).""" + from keepercommander.service.util.request_validation import RequestValidator + + request_data = {'filedata': {'some': 'data'}} + command = 'pam project import --filename=FILEDATA' + processed_command, temp_files = RequestValidator.process_file_data(request_data, command) + self.addCleanup(RequestValidator.cleanup_temp_files, temp_files) + + self.assertTrue(temp_files, 'expected a temp file to be created') + self.assertNotIn('FILEDATA', processed_command) + + request_temp_dir = os.path.dirname(temp_files[0]) + tokens = _tokens(processed_command) + self.assertIsNone( + Verifycommand.validate_service_mode_restrictions(tokens, request_temp_dir) + ) + + def test_option_values_yields_dash_leading_values(self): + is_file = Verifycommand._option_values + self.assertEqual(list(is_file(['generate', '--output', '-'], '--output')), ['-']) + self.assertEqual( + list(is_file(['pam', 'project', 'import', '-f', '-etc/passwd'], '-f')), + ['-etc/passwd'], + ) + + check = Verifycommand.validate_service_mode_restrictions + ban = 'Local filesystem access' + self.assertIn(ban, check(_tokens('pam project import -f -etc/passwd'))) + self.assertIn(ban, check(_tokens('pam project import --filename -etc/passwd'))) + + def test_config_file_blocked_config_base64_allowed(self): + check = Verifycommand.validate_service_mode_restrictions + self.assertIsNotNone( + check(_tokens('credential-provision --config=/tmp/cfg.yaml -c PAMUID')) + ) + self.assertIsNone( + check(_tokens('credential-provision --config-base64 dGVzdA== -c PAMUID')) + ) + def test_is_record_file_attachment_arg(self): is_file = Verifycommand._is_record_file_attachment_arg self.assertTrue(is_file('file=@/tmp/x')) diff --git a/unit-tests/service/test_throttle_response.py b/unit-tests/service/test_throttle_response.py index 361ea5173..f0747bf76 100644 --- a/unit-tests/service/test_throttle_response.py +++ b/unit-tests/service/test_throttle_response.py @@ -78,6 +78,44 @@ def test_parser_maps_throttled_text_to_429(self): 'Due to repeated attempts, your request has been throttled.', ) + def test_parser_treats_share_invitation_as_success(self): + result = KeeperResponseParser._parse_logging_based_command( + 'share-folder', + "Share invitation has been sent to 'user@example.com'\n" + "Please repeat this command when invitation is accepted.\n" + "User user@example.com not found", + ) + self.assertEqual(result['status'], 'success') + self.assertNotIn('error', result) + self.assertIn('Share invitation has been sent', str(result['message'])) + + def test_parser_treats_nsf_share_record_invitation_as_success(self): + result = KeeperResponseParser._parse_logging_based_command( + 'nsf-share-record', + "nsf-share-record: Share invitation has been sent to 'user@example.com'. " + "Please repeat this command once the invitation is accepted.", + ) + self.assertEqual(result['status'], 'success') + self.assertIn('Share invitation has been sent', str(result['message'])) + + def test_parser_keeps_throttle_precedence_over_invitation(self): + result = KeeperResponseParser._parse_logging_based_command( + 'share-folder', + "Share invitation has been sent to 'user@example.com'\n" + "throttled: Due to repeated attempts, your request has been throttled.", + ) + self.assertEqual(result['status_code'], 429) + self.assertEqual(result['result_code'], RESULT_THROTTLED) + + def test_parser_keeps_forbidden_precedence_over_invitation(self): + result = KeeperResponseParser._parse_logging_based_command( + 'share-record', + "Share invitation has been sent to 'user@example.com'\n" + "Permission denied", + ) + self.assertEqual(result['status'], 'error') + self.assertEqual(result.get('status_code'), 403) + def test_parser_does_not_treat_rate_limit_config_text_as_throttle(self): result = KeeperResponseParser._parse_logging_based_command( 'help', diff --git a/unit-tests/test_api.py b/unit-tests/test_api.py index 7601c1926..415057ed5 100644 --- a/unit-tests/test_api.py +++ b/unit-tests/test_api.py @@ -61,6 +61,37 @@ def test_search_shared_folders(self): sfs = api.search_shared_folders(params, 'INVALID') self.assertEqual(len(sfs), 0) + def test_search_nested_share_folders(self): + params = get_synced_params() + params.nested_share_folders = { + 'nsf_uid_1': {'name': 'Test Folder 1'}, + 'nsf_uid_2': {'name': 'Test Folder 2'}, + 'nsf_uid_3': {'name': 'Another Folder'}, + } + + # Empty search returns all NSF folders + nsfs = api.search_nested_share_folders(params, '') + self.assertEqual(len(nsfs), 3) + + # Token search matches name + nsfs = api.search_nested_share_folders(params, 'test') + self.assertEqual(len(nsfs), 2) + + # UID match + nsfs = api.search_nested_share_folders(params, 'nsf_uid_1') + self.assertEqual(len(nsfs), 1) + self.assertEqual(nsfs[0], ('nsf_uid_1', 'Test Folder 1')) + + # No match + nsfs = api.search_nested_share_folders(params, 'INVALID') + self.assertEqual(len(nsfs), 0) + + # Missing nested_share_folders attribute + params_no_nsf = get_synced_params() + delattr(params_no_nsf, 'nested_share_folders') + nsfs = api.search_nested_share_folders(params_no_nsf, '') + self.assertEqual(len(nsfs), 0) + def test_search_teams(self): params = get_synced_params() diff --git a/unit-tests/test_command_record.py b/unit-tests/test_command_record.py index d21e3927f..584f5172d 100644 --- a/unit-tests/test_command_record.py +++ b/unit-tests/test_command_record.py @@ -309,6 +309,39 @@ def test_shared_list_filters_by_roe_eligible(self): cmd.execute(params, roe_eligible=True) mock_print.assert_called() + def test_shared_list_nsf_only_when_roe_eligible(self): + params = get_synced_params() + params.nested_share_folders = {'nsf_uid_1': {'name': 'Test NSF Folder'}} + params.shared_folder_cache = {} # Clear classic folders to isolate NSF + cmd = record.RecordListSfCommand() + + # Without roe_eligible, NSF should not be included + with mock.patch('keepercommander.commands.base.dump_report_data') as dump: + cmd.execute(params, roe_eligible=False) + self.assertEqual(dump.call_count, 0, "NSF should not be searched when roe_eligible=False (no results)") + + # With roe_eligible, NSF should be included if it has PAM rotation + with mock.patch('keepercommander.commands.base.dump_report_data') as dump: + with mock.patch( + 'keepercommander.vault_extensions.nested_share_folder_has_pam_user_with_rotation', + return_value=True): + cmd.execute(params, roe_eligible=True) + rows = dump.call_args[0][0] + self.assertEqual(len(rows), 1, "NSF should be included when roe_eligible=True and has PAM rotation") + self.assertEqual(rows[0][2], 'Nested', "NSF rows should have folder_type='Nested'") + + def test_shared_list_folder_type_column(self): + params = get_synced_params() + cmd = record.RecordListSfCommand() + + with mock.patch( + 'keepercommander.vault_extensions.shared_folder_has_pam_user_with_rotation', + return_value=True): + with mock.patch('keepercommander.commands.base.dump_report_data') as dump: + cmd.execute(params, roe_eligible=True, format='json') + headers = dump.call_args[0][1] + self.assertIn('folder_type', headers, "JSON output should include folder_type column") + def test_team_list_command(self): params = get_synced_params() cmd = record.RecordListTeamCommand() diff --git a/unit-tests/test_nested_share_folder.py b/unit-tests/test_nested_share_folder.py index 7c912c6bc..7eed51d12 100644 --- a/unit-tests/test_nested_share_folder.py +++ b/unit-tests/test_nested_share_folder.py @@ -1393,11 +1393,14 @@ def test_share_folder_rejects_grant_to_owner(self, mock_grant): @patch('keepercommander.nested_share_folder.folder_api.grant_folder_access_v3') def test_share_folder_invite_message_uses_command_prefix(self, mock_grant): + import keepercommander.nested_share_folder as nsf from keepercommander.commands.nested_share_folder import NestedShareFolderShareCommand + from keepercommander.nested_share_folder.common import ShareInviteSentError + nsf.__dict__.pop('grant_folder_access_v3', None) fuid, fobj = _make_folder() email = 'user@example.com' - mock_grant.side_effect = ValueError( + mock_grant.side_effect = ShareInviteSentError( f"Share invitation has been sent to '{email}'. " "Please repeat this command once the invitation is accepted.") @@ -1410,6 +1413,65 @@ def test_share_folder_invite_message_uses_command_prefix(self, mock_grant): self.assertIn('nsf-share-folder: Share invitation has been sent', output) self.assertNotIn("User '", output) + @patch('keepercommander.nested_share_folder.folder_api.grant_folder_access_v3') + def test_share_folder_no_relationship_still_fails(self, mock_grant): + import keepercommander.nested_share_folder as nsf + from keepercommander.commands.nested_share_folder import NestedShareFolderShareCommand + + nsf.__dict__.pop('grant_folder_access_v3', None) + fuid, fobj = _make_folder() + mock_grant.side_effect = ValueError( + "No sharing relationship with 'user@example.com'. " + "Please invite them to share first, then repeat this command.") + + cmd = NestedShareFolderShareCommand() + with self.assertRaises(CommandError) as ctx: + cmd.execute(_make_params(nested_share_folders={fuid: fobj}), + folder=[fuid], user=['user@example.com'], action='grant', role='viewer') + self.assertIn('No sharing relationship', str(ctx.exception)) + + @patch('keepercommander.nested_share_folder.record_api.share_record_v3') + def test_share_record_invite_message_logged_as_warning(self, mock_share): + import keepercommander.nested_share_folder as nsf + from keepercommander.commands.nested_share_folder import NestedShareRecordShareCommand + from keepercommander.nested_share_folder.common import ShareInviteSentError + + nsf.__dict__.pop('share_record_v3', None) + ruid, robj = _make_record() + email = 'user@example.com' + mock_share.side_effect = ShareInviteSentError( + f"Share invitation has been sent to '{email}'. " + "Please repeat this command once the invitation is accepted.") + + cmd = NestedShareRecordShareCommand() + with self.assertLogs(level='WARNING') as logs, \ + mock.patch.object(NestedShareRecordShareCommand, '_get_direct_user_share', + return_value=None): + cmd.execute(_make_params(nested_share_records={ruid: robj}), + record=ruid, email=[email], action='grant', role='viewer') + + output = '\n'.join(logs.output) + self.assertIn('nsf-share-record: Share invitation has been sent', output) + + @patch('keepercommander.nested_share_folder.record_api.share_record_v3') + def test_share_record_no_relationship_still_fails(self, mock_share): + import keepercommander.nested_share_folder as nsf + from keepercommander.commands.nested_share_folder import NestedShareRecordShareCommand + + nsf.__dict__.pop('share_record_v3', None) + ruid, robj = _make_record() + mock_share.side_effect = ValueError( + "No sharing relationship with 'user@example.com'. " + "Please invite them to share first, then repeat this command.") + + cmd = NestedShareRecordShareCommand() + with mock.patch.object(NestedShareRecordShareCommand, '_get_direct_user_share', + return_value=None): + with self.assertRaises(CommandError) as ctx: + cmd.execute(_make_params(nested_share_records={ruid: robj}), + record=ruid, email=['user@example.com'], action='grant', role='viewer') + self.assertIn('No sharing relationship', str(ctx.exception)) + def test_share_record_roe_rejects_non_grant(self): from keepercommander.commands.nested_share_folder import NestedShareRecordShareCommand ruid, robj = _make_record()