diff --git a/keepercommander/commands/discoveryrotation.py b/keepercommander/commands/discoveryrotation.py index c8c3bd13b..f011378da 100644 --- a/keepercommander/commands/discoveryrotation.py +++ b/keepercommander/commands/discoveryrotation.py @@ -75,13 +75,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 +461,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(), 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/record_commands.py b/keepercommander/commands/nested_share_folder/record_commands.py index faf277ce1..4748ac7f7 100644 --- a/keepercommander/commands/nested_share_folder/record_commands.py +++ b/keepercommander/commands/nested_share_folder/record_commands.py @@ -25,7 +25,7 @@ from ..record_edit import RecordEditMixin, record_fields_description from ...enforcement import PasswordComplexityEnforcer, RecordTypeEnforcer from ...error import CommandError -from ... import nested_share_folder as _nsf, vault, vault_extensions +from ... import generator, nested_share_folder as _nsf, vault, vault_extensions from .helpers import ( resolve_folder_uid, command_error_handler, check_result, check_record_edit_permission, check_record_delete_permission, @@ -54,26 +54,6 @@ def _parse_field_specs(raw_fields, cmd_name): return record_fields, attachments -def _apply_password_policy(editor, params, source, force): - # TypedRecord (typed add/update) or v3 dict (legacy add). validate_record accepts both. - pw_failures = PasswordComplexityEnforcer.validate_record(params, source) - for failure in pw_failures: - editor.on_warning(failure) - if pw_failures and not force: - editor.on_warning('Use --force to bypass password policy warnings.') - - -def _should_stop_after_warnings(editor, force): - if not editor.warnings: - return False - for w in editor.warnings: - logging.warning(w) - if not force: - return True - editor.warnings.clear() - return False - - def _unsupported_attachment_warning(attachments, cmd_name): if not attachments: return @@ -124,11 +104,14 @@ def execute(self, params, **kwargs): folder_uid = self._resolve_folder(params, kwargs.get('folder_uid')) data = self._build_record_data( - params, record_type, title, notes, record_fields, kwargs.get('force')) + params, record_type, title, notes, record_fields) if self.abort_if_errors(): return - if _should_stop_after_warnings(self, kwargs.get('force')): - return + if self.warnings: + for w in self.warnings: + logging.warning(w) + if not kwargs.get('force'): + return if add_attachments: _unsupported_attachment_warning(add_attachments, 'nsf-record-add') @@ -156,14 +139,13 @@ def _resolve_folder(params, folder_input): ensure_nested_share_folder(params, uid, 'nsf-record-add', identifier=folder_input) return uid - def _build_record_data(self, params, record_type, title, notes, record_fields, force=False): + def _build_record_data(self, params, record_type, title, notes, record_fields): if record_type in ('legacy', 'general'): record = vault.PasswordRecord() self.assign_legacy_fields(record, record_fields) record.title = title record.notes = self.validate_notes(notes or '') data = self._legacy_to_data(record, title) - _apply_password_policy(self, params, data, force) return data rt_fields = self.get_record_type_fields(params, record_type) @@ -183,7 +165,6 @@ def _build_record_data(self, params, record_type, title, notes, record_fields, f self.assign_typed_fields(record, record_fields) record.title = title record.notes = self.validate_notes(notes or '') - _apply_password_policy(self, params, record, force) return self._typed_to_data(record, title) @staticmethod @@ -231,6 +212,34 @@ def __init__(self): def get_parser(self): return nested_share_record_update_parser + def _resolve_field_value(self, parsed): + raw = parsed.value + if not raw: + return raw + + action_params = [] + if self.is_json_value(raw, action_params): + return action_params[0] if action_params else None + action_params.clear() + if self.is_generate_value(raw, action_params): + if self.warn_wrong_password_gen_field(parsed): + return None + if parsed.type == 'password': + algorithm, _ = generator.resolve_gen_password_algorithm(action_params) + password, gen_error = self.generate_password(action_params, policy=self._password_policy) + if gen_error: + self.on_error(gen_error) + return None + if password is not None: + self.validate_generated_password(password, algorithm) + return password + if parsed.type in ('oneTimeCode', 'otp'): + return self.generate_totp_url() + return raw + action_params.clear() + if self.is_base64_value(raw, action_params): + return action_params[0] if action_params else None + return raw def execute(self, params, **kwargs): if kwargs.get('syntax_help'): print(record_fields_description) @@ -286,9 +295,12 @@ def execute(self, params, **kwargs): if self.abort_if_errors(): continue - _apply_password_policy(self, params, record, force) - if _should_stop_after_warnings(self, force): - continue + if self.warnings: + for w in self.warnings: + logging.warning(w) + if not force: + continue + self.warnings.clear() result = self._send_typed_update(params, record_uid, record) # API syncs on RS_OUT_OF_SYNC but does not retry a full data @@ -303,9 +315,12 @@ def execute(self, params, **kwargs): record_type, rt_fields, record_fields) if self.abort_if_errors(): continue - _apply_password_policy(self, params, record, force) - if _should_stop_after_warnings(self, force): - continue + if self.warnings: + for w in self.warnings: + logging.warning(w) + if not force: + continue + self.warnings.clear() result = self._send_typed_update(params, record_uid, record) check_result(result, 'nsf-record-update') updated += 1 @@ -350,7 +365,6 @@ def _typed_record_from_uid(self, params, record_uid): record = vault.TypedRecord() record.load_record_data(existing) return record - @staticmethod def _load_record_data(params, record_uid): # type: (Any, str) -> Optional[Dict] rec = params.record_cache.get(record_uid) or {} 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_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_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_edit.py b/keepercommander/commands/record_edit.py index 4b6bd2da3..e4f233105 100644 --- a/keepercommander/commands/record_edit.py +++ b/keepercommander/commands/record_edit.py @@ -266,6 +266,16 @@ def abort_if_errors(self): def on_info(self, message): logging.info(message) + def validate_generated_password(self, password, algorithm): + """Validate a generated password with the correct algorithm flag.""" + if not password or not self._password_policy: + return + allow_passphrase = algorithm == 'passphrase' + failures = PasswordComplexityEnforcer.validate_password( + password, self._password_policy, allow_passphrase_fallback=allow_passphrase) + for failure in failures: + self.on_warning(failure) + def warn_wrong_password_gen_field(self, parsed_field): # type: (ParsedFieldValue) -> bool """Warn when $GEN is used with a label like Password= instead of password=.""" @@ -328,10 +338,12 @@ def assign_legacy_fields(self, record, fields): elif parsed_field.type == 'password': action_params.clear() if self.is_generate_value(parsed_field.value, action_params): + algorithm, _ = generator.resolve_gen_password_algorithm(action_params) password, gen_error = self.generate_password(action_params, policy=self._password_policy) if gen_error: self.on_error(gen_error) elif password is not None: + self.validate_generated_password(password, algorithm) record.password = password elif self.is_base64_value(parsed_field.value, action_params): if action_params: @@ -669,10 +681,13 @@ def assign_typed_fields(self, record, fields): action_params = [] if self.is_generate_value(parsed_field.value, action_params): if record_field.type == 'password': + algorithm, _ = generator.resolve_gen_password_algorithm(action_params) value, gen_error = self.generate_password(action_params, policy=self._password_policy) if gen_error: self.on_error(gen_error) continue + if value is not None: + self.validate_generated_password(value, algorithm) elif record_field.type in ('oneTimeCode', 'otp'): value = self.generate_totp_url() elif record_field.type in ('keyPair', 'privateKey'): @@ -683,6 +698,8 @@ def assign_typed_fields(self, record, fields): if gen_error: self.on_error(gen_error) continue + if passphrase: + self.validate_generated_password(passphrase, 'password') key_type = next((x for x in action_params if x in ('rsa', 'ec', 'ed25519')), 'rsa') value = self.generate_key_pair(key_type, passphrase) if passphrase: @@ -981,12 +998,6 @@ def execute(self, params, **kwargs): record.title = title record.notes = self.validate_notes(kwargs.get('notes') or '') - pw_failures = PasswordComplexityEnforcer.validate_record(params, record) - for f in pw_failures: - self.on_warning(f) - if pw_failures and not kwargs.get('force'): - self.on_warning('Use --force to bypass password policy warnings.') - ignore_warnings = kwargs.get('force') is True if len(self.warnings) > 0: for warning in self.warnings: @@ -1370,13 +1381,6 @@ def execute(self, params, **kwargs): if self.abort_if_errors(): return - if isinstance(record, vault.TypedRecord): - pw_failures = PasswordComplexityEnforcer.validate_record(params, record) - for f in pw_failures: - self.on_warning(f) - if pw_failures and not kwargs.get('force'): - self.on_warning('Use --force to bypass password policy warnings.') - ignore_warnings = kwargs.get('force') is True if len(self.warnings) > 0: for warning in self.warnings: diff --git a/keepercommander/commands/recordv3.py b/keepercommander/commands/recordv3.py index f286800b6..b55d55a70 100644 --- a/keepercommander/commands/recordv3.py +++ b/keepercommander/commands/recordv3.py @@ -142,6 +142,22 @@ def get_password_from_rules(generate_rules, generate_length): return kpg.generate() +def enforce_generated_password_policy(params, data, command, generated, manual_password=None, force=False): + # Skip if user explicitly provided a password (overrides generation) + if not generated or manual_password: + return + pw_failures = PasswordComplexityEnforcer.validate_record( + params, data, allow_passphrase_fallback=False) + if pw_failures: + for failure in pw_failures: + logging.warning(bcolors.WARNING + failure + bcolors.ENDC) + if not force: + raise CommandError( + command, + 'Password does not meet enterprise complexity policy. ' + 'Pass --force to bypass these warnings.') + + class RecordAddCommand(Command, recordv2.RecordUtils): def get_parser(self): return add_parser @@ -404,21 +420,15 @@ def GCM_TAG_LEN(): return 16 # For compatibility w/ legacy: --password overrides --generate AND --generate overrides dataJSON/option # dataJSON/option < kwargs: --generate < kwargs: --password - password = kwargs.get('password') + manual_password = kwargs.get('password') + password = manual_password if not password and (kwargs.get('generate') or kwargs.get('generate_rules') or kwargs.get('generate_length')): password = get_password_from_rules(kwargs.get('generate_rules'), kwargs.get('generate_length')) if password: data = recordv3.RecordV3.update_password(password, data, recordv3.RecordV3.get_record_type_definition(params, data)) - pw_failures = PasswordComplexityEnforcer.validate_record(params, data) - if pw_failures: - for f in pw_failures: - logging.warning(bcolors.WARNING + f + bcolors.ENDC) - if not kwargs.get('force'): - raise CommandError( - 'add', - 'Password does not meet enterprise complexity policy. ' - 'Pass --force to bypass these warnings.') + generated = bool(kwargs.get('generate') or kwargs.get('generate_rules') or kwargs.get('generate_length')) + enforce_generated_password_policy(params, data, 'add', generated, manual_password=manual_password, force=kwargs.get('force')) record_uid = api.generate_record_uid() logging.debug('Generated Record UID: %s', record_uid) @@ -653,22 +663,16 @@ def execute(self, params, **kwargs): # For compatibility w/ legacy: --password overides --generate AND --generate overrides dataJSON/option # dataJSON/option < kwargs: --generate < kwargs: --password - password = kwargs.get('password') + manual_password = kwargs.get('password') + password = manual_password if not password and generate: password = get_password_from_rules(kwargs.get('generate_rules'), kwargs.get('generate_length')) if password: record.password = password data = recordv3.RecordV3.update_password(password, data, recordv3.RecordV3.get_record_type_definition(params, data)) - pw_failures = PasswordComplexityEnforcer.validate_record(params, data) - if pw_failures: - for f in pw_failures: - logging.warning(bcolors.WARNING + f + bcolors.ENDC) - if not kwargs.get('force'): - raise CommandError( - 'edit', - 'Password does not meet enterprise complexity policy. ' - 'Pass --force to bypass these warnings.') + generated = bool(kwargs.get('generate') or kwargs.get('generate_rules') or kwargs.get('generate_length')) + enforce_generated_password_policy(params, data, 'edit', generated, manual_password=manual_password, force=kwargs.get('force')) data_dict = json.loads(data) changed = rdata_dict != data_dict 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/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/enforcement.py b/keepercommander/enforcement.py index 2e3cd0a43..a53e824eb 100644 --- a/keepercommander/enforcement.py +++ b/keepercommander/enforcement.py @@ -505,7 +505,7 @@ def validate_passphrase(cls, password, policy): # type: (str, Dict[str, Any]) return failures @classmethod - def validate_password(cls, password, policy): # type: (str, Dict[str, Any]) -> List[str] + def validate_password(cls, password, policy, allow_passphrase_fallback=True): # type: (str, Dict[str, Any], bool) -> List[str] failures = [] # type: List[str] if not policy or not isinstance(password, str) or not password: return failures @@ -540,7 +540,7 @@ def validate_password(cls, password, policy): # type: (str, Dict[str, Any]) -> return [] # Vault re-validates as a passphrase when random password rules fail. - if policy.get('passphrase-allow') is False: + if not allow_passphrase_fallback or policy.get('passphrase-allow') is False: return failures passphrase_failures = cls.validate_passphrase(password, policy) @@ -550,7 +550,7 @@ def validate_password(cls, password, policy): # type: (str, Dict[str, Any]) -> return passphrase_failures @classmethod - def validate_record(cls, params, source): # type: (KeeperParams, Any) -> List[str] + def validate_record(cls, params, source, allow_passphrase_fallback=True): # type: (KeeperParams, Any, bool) -> List[str] """Return policy violations across all password fields in `source`. `source` may be a vault.TypedRecord, a v3 record-data dict, or a JSON @@ -562,7 +562,7 @@ def validate_record(cls, params, source): # type: (KeeperParams, Any) -> List[ return [] failures = [] # type: List[str] for pw in cls._extract_passwords(source): - failures.extend(cls.validate_password(pw, policy)) + failures.extend(cls.validate_password(pw, policy, allow_passphrase_fallback)) return failures @staticmethod 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/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/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/unit-tests/pam/test_pam_debug_nsf.py b/unit-tests/pam/test_pam_debug_nsf.py index b68666192..855327b0f 100644 --- a/unit-tests/pam/test_pam_debug_nsf.py +++ b/unit-tests/pam/test_pam_debug_nsf.py @@ -9,9 +9,7 @@ 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 @@ -74,47 +72,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/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_nested_share_folder.py b/unit-tests/test_nested_share_folder.py index 7c912c6bc..ba9ebbad8 100644 --- a/unit-tests/test_nested_share_folder.py +++ b/unit-tests/test_nested_share_folder.py @@ -452,7 +452,7 @@ def setUp(self): def tearDown(self): mock.patch.stopall() - @patch('keepercommander.nested_share_folder.record_api.create_record_v3') + @patch('keepercommander.commands.nested_share_folder.record_commands._nsf.create_record_v3') def test_add_record(self, mock_create): from keepercommander.commands.nested_share_folder import NestedShareRecordAddCommand mock_create.return_value = { @@ -484,8 +484,12 @@ def test_add_record_rejects_restricted_record_type(self, mock_create): mock_create.assert_not_called() @patch('keepercommander.commands.nested_share_folder.record_commands._nsf.create_record_v3') - def test_add_record_rejects_weak_password_without_force(self, mock_create): + def test_add_record_allows_manual_weak_password(self, mock_create): from keepercommander.commands.nested_share_folder import NestedShareRecordAddCommand + mock_create.return_value = { + 'record_uid': utils.generate_uid(), 'status': 'SUCCESS', + 'message': '', 'success': True, 'revision': 1, + } fuid, fobj = _make_folder() params = _make_params( nested_share_folders={fuid: fobj}, @@ -504,7 +508,7 @@ def test_add_record_rejects_weak_password_without_force(self, mock_create): cmd = NestedShareRecordAddCommand() cmd.execute(params, title='Weak', record_type='general', fields=['password=abc'], force=False) - mock_create.assert_not_called() + mock_create.assert_called_once() @patch('keepercommander.commands.nested_share_folder.record_commands._nsf.create_record_v3') def test_add_record_allows_weak_password_with_force(self, mock_create): @@ -595,8 +599,12 @@ def test_update_record_rejects_restricted_record_type(self, mock_perm, mock_upda @patch('keepercommander.commands.nested_share_folder.record_commands._nsf.update_record_v3') @patch('keepercommander.commands.nested_share_folder.helpers.check_record_edit_permission') - def test_update_record_rejects_weak_password_without_force(self, mock_perm, mock_update): + def test_update_record_allows_manual_weak_password(self, mock_perm, mock_update): from keepercommander.commands.nested_share_folder import NestedShareRecordUpdateCommand + mock_update.return_value = { + 'record_uid': 'x', 'status': 'SUCCESS', + 'message': '', 'success': True, 'revision': 2, + } ruid, robj = _make_record() params = _make_params( nested_share_records={ruid: robj}, @@ -618,7 +626,7 @@ def test_update_record_rejects_weak_password_without_force(self, mock_perm, mock ) cmd = NestedShareRecordUpdateCommand() cmd.execute(params, record_uids=[ruid], fields=['password=abc'], force=False) - mock_update.assert_not_called() + mock_update.assert_called_once() @patch('keepercommander.commands.nested_share_folder.record_commands._nsf.update_record_v3') @patch('keepercommander.commands.nested_share_folder.helpers.check_record_edit_permission') @@ -957,7 +965,7 @@ def test_update_sets_sync_data_after_partial_batch_failure(self, mock_perm, mock @patch('keepercommander.commands.nested_share_folder.record_commands._nsf.update_record_v3') @patch('keepercommander.commands.nested_share_folder.helpers.check_record_edit_permission') - def test_update_skips_warned_record_and_continues_batch(self, mock_perm, mock_update): + def test_update_allows_weak_manual_passwords_in_batch(self, mock_perm, mock_update): from keepercommander.commands.nested_share_folder import NestedShareRecordUpdateCommand policy = json.dumps([{ 'length': 12, @@ -980,11 +988,11 @@ def test_update_skips_warned_record_and_continues_batch(self, mock_perm, mock_up }), } params.enforcements = {'jsons': [{'key': 'generated_password_complexity', 'value': policy}]} + self._stub_update(mock_update, ruid1) self._stub_update(mock_update, ruid2) NestedShareRecordUpdateCommand().execute( params, record_uids=[ruid1, ruid2], title='Updated', fields=[], force=False) - self.assertEqual(mock_update.call_count, 1) - self.assertEqual(mock_update.call_args.kwargs['record_uid'], ruid2) + self.assertEqual(mock_update.call_count, 2) self.assertTrue(params.sync_data) @patch('keepercommander.commands.nested_share_folder.record_commands._nsf.update_record_v3') @@ -1017,8 +1025,12 @@ def test_update_empty_file_spec_warns_as_removal(self, mock_perm, mock_update): mock_update.assert_not_called() @patch('keepercommander.commands.nested_share_folder.record_commands._nsf.create_record_v3') - def test_add_rejects_weak_custom_password_without_force(self, mock_create): + def test_add_allows_weak_custom_password_without_force(self, mock_create): from keepercommander.commands.nested_share_folder import NestedShareRecordAddCommand + mock_create.return_value = { + 'record_uid': utils.generate_uid(), 'status': 'SUCCESS', + 'message': '', 'success': True, 'revision': 1, + } fuid, fobj = _make_folder() params = _make_params( nested_share_folders={fuid: fobj}, @@ -1039,7 +1051,7 @@ def test_add_rejects_weak_custom_password_without_force(self, mock_create): return_value=[{'$ref': 'login'}, {'$ref': 'password'}]): cmd.execute(params, title='Weak custom', record_type='login', fields=['c.password.AppSecret=abc'], force=False) - mock_create.assert_not_called() + mock_create.assert_called_once() @patch('keepercommander.commands.nested_share_folder.record_commands._nsf.create_record_v3') def test_add_unescapes_notes(self, mock_create): @@ -1393,11 +1405,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 +1425,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() diff --git a/unit-tests/test_passphrase_enforcement.py b/unit-tests/test_passphrase_enforcement.py index 4093ba7de..9bc0f6471 100644 --- a/unit-tests/test_passphrase_enforcement.py +++ b/unit-tests/test_passphrase_enforcement.py @@ -1,6 +1,10 @@ +from types import SimpleNamespace from unittest import TestCase +from keepercommander.commands.record_edit import RecordEditMixin +from keepercommander.commands.recordv3 import enforce_generated_password_policy from keepercommander.enforcement import PasswordComplexityEnforcer +from keepercommander.error import CommandError STRICT_RANDOM_POLICY = { @@ -82,3 +86,59 @@ def test_random_password_still_validated(self): password = 'ABCDE123!!!' failures = PasswordComplexityEnforcer.validate_password(password, STRICT_RANDOM_POLICY) self.assertEqual(failures, []) + + def test_invalid_random_password_keeps_random_policy_errors(self): + policy = dict(STRICT_RANDOM_POLICY) + policy.update({'length': 20, 'passphrase-length': 5, 'passphrase-separator': '!'}) + failures = PasswordComplexityEnforcer.validate_password( + 'ABC123!!', policy, allow_passphrase_fallback=False) + self.assertTrue(any('Password must be at least 20 characters' in f for f in failures)) + self.assertFalse(any('Passphrase contains' in f for f in failures)) + + +class TestGeneratedPasswordPolicyWarnings(TestCase): + + def _params(self): + return SimpleNamespace(enforcements={ + 'jsons': [{ + 'key': 'generated_password_complexity', + 'value': '{"length": 20, "passphrase-allow": true, "passphrase-length": 5}', + }], + }) + + def test_generated_random_password_fails_when_too_short(self): + mixin = RecordEditMixin() + mixin._password_policy = PasswordComplexityEnforcer.get_policy(self._params()) + mixin.validate_generated_password('pass', 'password') + self.assertTrue(any('Password must be at least' in w for w in mixin.warnings)) + + def test_generated_passphrase_validates_with_passphrase_fallback(self): + mixin = RecordEditMixin() + mixin._password_policy = PasswordComplexityEnforcer.get_policy(self._params()) + mixin.validate_generated_password('pass', 'passphrase') + # Passphrase validation is looser, may not fail on short input + self.assertTrue(isinstance(mixin.warnings, list)) + + +class TestV3GeneratedPasswordPolicy(TestCase): + + def _params(self): + return SimpleNamespace(enforcements={ + 'jsons': [{ + 'key': 'generated_password_complexity', + 'value': '{"length": 20}', + }], + }) + + def _record(self): + return {'fields': [{'type': 'password', 'value': ['pass']}]} + + def test_generated_password_is_rejected_when_policy_fails(self): + with self.assertRaises(CommandError): + enforce_generated_password_policy( + self._params(), self._record(), 'add', generated=True, manual_password=None) + + def test_explicit_password_overrides_generation_for_policy(self): + enforce_generated_password_policy( + self._params(), self._record(), 'edit', generated=True, + manual_password='explicit', force=False) diff --git a/unit-tests/test_passphrase_generator.py b/unit-tests/test_passphrase_generator.py index 8cf9b40e2..522fa5199 100644 --- a/unit-tests/test_passphrase_generator.py +++ b/unit-tests/test_passphrase_generator.py @@ -205,6 +205,22 @@ def test_invalid_gen_algorithm_aborts_record_add(self): self.assertEqual(len(cmd.errors), 1) self.assertIn('passphrase', cmd.errors[0]) + def test_encrypted_key_pair_validates_passphrase(self): + from keepercommander.commands.record_edit import RecordAddCommand, ParsedFieldValue + from keepercommander import vault + cmd = RecordAddCommand() + record = vault.TypedRecord() + record.type_name = 'sshKeys' + record.fields.append(vault.TypedField.new_field('keyPair', '', '')) + with mock.patch.object(cmd, 'generate_password', return_value=('weak', None)), \ + mock.patch.object(cmd, 'generate_key_pair', return_value={'privateKey': 'key'}), \ + mock.patch.object(cmd, 'validate_generated_password') as mock_validate: + cmd.assign_typed_fields(record, [ + ParsedFieldValue('', 'keyPair', '', '$GEN:enc'), + ]) + # Verify validate_generated_password was called for the passphrase + mock_validate.assert_called_once_with('weak', 'password') + def test_invalid_passphrase_separator_aborts_generation(self): from keepercommander.commands.record_edit import RecordEditMixin result, error = RecordEditMixin.generate_password(