diff --git a/keepercommander/commands/folder.py b/keepercommander/commands/folder.py index ade0054c6..78482639f 100644 --- a/keepercommander/commands/folder.py +++ b/keepercommander/commands/folder.py @@ -230,9 +230,8 @@ def _load_record_for_ls(params, uid): dj = nsf_record_data[uid]['data_json'] rec = vault.TypedRecord(version=3) rec.record_uid = uid - rec.title = dj.get('title', uid) - rec.type_name = dj.get('type', '') rec.load_record_data(dj, None) + rec.title = rec.title or uid return rec return None diff --git a/keepercommander/commands/pam_saas/config.py b/keepercommander/commands/pam_saas/config.py index 0d0ccc055..02456577b 100644 --- a/keepercommander/commands/pam_saas/config.py +++ b/keepercommander/commands/pam_saas/config.py @@ -75,9 +75,37 @@ class PAMActionSaasConfigCommand(PAMGatewayActionDiscoverCommandBase): action='store', help='Shared folder or Nested Share Folder UID/name to store SaaS configuration.') + parser.add_argument('--format', '-f', required=False, dest='format', action='store', + choices=['text', 'json'], default='text', + help='Output format: text (default) or json.') + def get_parser(self): return PAMActionSaasConfigCommand.parser + @staticmethod + def _list_to_json(plugins: dict[str, SaasCatalog]) -> str: + """Convert plugins list to JSON format.""" + result = [] + sorted_catalog = {} # type: dict[str, SaasCatalog] + if plugins: + sorted_catalog = dict(sorted(plugins.items(), key=lambda i: i[1].name)) + + for _, plugin in sorted_catalog.items(): + plugin_dict = { + "name": plugin.name, + "type": plugin.type, + "summary": plugin.summary or "", + "author": plugin.author or "", + "email": plugin.email or "", + "readme": plugin.readme or "", + "installed": plugin.installed, + "using": len(plugin.used_by) > 0, + "used_by_count": len(plugin.used_by), + } + result.append(plugin_dict) + + return json.dumps(result, indent=2) + @staticmethod def _show_list(plugins: dict[str, SaasCatalog]): @@ -112,6 +140,48 @@ def _show_list(plugins: dict[str, SaasCatalog]): row = f" * {name}{desc} - {summary}" print(row) + @staticmethod + def _plugin_info_to_json(plugin: SaasCatalog) -> str: + """Convert plugin info to JSON format.""" + required_fields = [] + optional_fields = [] + + for field in plugin.fields: + field_dict = { + "label": field.label, + "description": field.desc, + # Match get --format json behavior: display type as "secret" for sensitive fields + "type": "secret" if field.is_secret else (field.type or "text"), + "required": field.required, + } + if field.default_value is not None: + field_dict["default_value"] = field.default_value + if field.enum_values: + field_dict["enum_values"] = [ + {"value": ev.value, "description": ev.desc or ""} + for ev in field.enum_values + ] + + if field.required: + required_fields.append(field_dict) + else: + optional_fields.append(field_dict) + + result = { + "name": plugin.name, + "type": plugin.type, + "author": plugin.author or "", + "email": plugin.email or "", + "summary": plugin.summary or "", + "readme": plugin.readme or "", + "installed": plugin.installed, + "allows_remote_management": plugin.allows_remote_management or False, + "required_fields": required_fields, + "optional_fields": optional_fields, + } + + return json.dumps(result, indent=2) + @staticmethod def _show_plugin_info(plugin: SaasCatalog): print("") @@ -295,6 +365,7 @@ def execute(self, params: KeeperParams, **kwargs): do_create = kwargs.get("do_create", False) # type: bool do_update = kwargs.get("do_update", False) # type: bool shared_folder_uid = kwargs.get("shared_folder_uid") # type: str + format_option = kwargs.get("format", "text") # type: str use_plugin = kwargs.get("plugin") # type: str | None gateway = kwargs.get("gateway") # type: str @@ -317,18 +388,27 @@ def execute(self, params: KeeperParams, **kwargs): ) if do_list: - self._show_list(plugins) + if format_option == "json": + print(self._list_to_json(plugins)) + else: + self._show_list(plugins) elif use_plugin is not None: if use_plugin not in plugins: - print("") - print(f"{bcolors.FAIL}Cannot find '{use_plugin}' in the catalog.{bcolors.ENDC}") + if format_option == "json": + print(json.dumps({"error": f"Cannot find '{use_plugin}' in the catalog."})) + else: + print("") + print(f"{bcolors.FAIL}Cannot find '{use_plugin}' in the catalog.{bcolors.ENDC}") return plugin = plugins[use_plugin] if do_info: - self._show_plugin_info(plugin=plugin) + if format_option == "json": + print(self._plugin_info_to_json(plugin=plugin)) + else: + self._show_plugin_info(plugin=plugin) elif do_create: diff --git a/keepercommander/commands/pedm/pedm_admin.py b/keepercommander/commands/pedm/pedm_admin.py index 55008249c..30c69c493 100644 --- a/keepercommander/commands/pedm/pedm_admin.py +++ b/keepercommander/commands/pedm/pedm_admin.py @@ -18,6 +18,7 @@ from cryptography.hazmat.primitives import serialization from ... import crypto, constants, utils, api, vault +from ...display import bcolors from ...params import KeeperParams from ...pedm import admin_plugin, pedm_shared, admin_types, admin_storage from ...proto import NotificationCenter_pb2, pedm_pb2 @@ -58,7 +59,12 @@ def resolve_single_deployment(pedm: admin_plugin.PedmPlugin, deployment_name: An return deployments[0] @staticmethod - def resolve_existing_policies(pedm: admin_plugin.PedmPlugin, policy_names: Any) -> List[admin_types.PedmPolicy]: + def resolve_existing_policies( + pedm: admin_plugin.PedmPlugin, + policy_names: Any, + *, + missing_names: Optional[Set[str]] = None, + ) -> List[admin_types.PedmPolicy]: found_policies: Dict[str, admin_types.PedmPolicy] = {} p: Optional[admin_types.PedmPolicy] if isinstance(policy_names, list): @@ -79,13 +85,19 @@ def resolve_existing_policies(pedm: admin_plugin.PedmPlugin, policy_names: Any) isinstance(x.data.get('PolicyName'), str) and x.data['PolicyName'].lower() == l_name] if len(matches) == 0: - raise base.CommandError(f'Policy "{policy_name}" is not found') + if missing_names is None: + raise base.CommandError(f'Policy "{policy_name}" is not found') + else: + logging.warning(f'Policy "{policy_name}" is not found') + missing_names.add(policy_name) + continue if len(matches) > 1: raise base.CommandError(f'Policy "{policy_name}" is not unique. Please use Policy UID') found_policies[matches[0].policy_uid] = matches[0] if len(found_policies) == 0: - raise base.CommandError('No policies were found') + if missing_names is None or len(missing_names) == 0: + raise base.CommandError('No policies were found') return list(found_policies.values()) @staticmethod @@ -219,12 +231,19 @@ def __init__(self): subparsers = parser.add_subparsers(title='Directory Type', dest='auth_type', required=True, help='Authentication method') record_parser = subparsers.add_parser('record', help='Connection parameters from Keeper record') record_parser.add_argument('record_uid', help='Keeper record UID') + record_parser.add_argument('--insecure', dest='insecure', action='store_true', + help='Disable TLS certificate validation for Active Directory (LDAPS) connection. ' + 'Insecure: do not use on untrusted networks.') + record_parser.add_argument('--ad-ca-cert', dest='ca_cert', action='store', + help='Path to PEM file with CA certificate(s) to validate the Active Directory ' + '(LDAPS) server certificate against, when it is not issued by a CA already ' + 'trusted by this system.') azure_parser = subparsers.add_parser('azure', help='Connect via Azure AD') azure_parser.add_argument('--tenant-id', dest='tenant_id', required=True) azure_parser.add_argument('--client-id', dest='client_id', required=True) azure_parser.add_argument('--client-secret', dest='client_secret') - azure_parser.add_argument('--azure-cloud', dest='azure_cloud', choices=['US', 'GOV', 'CN', 'EU'], + azure_parser.add_argument('--azure-cloud', dest='azure_cloud', choices=['US', 'GOV', 'CN'], help='Azure cloud (AzureCloud, AzureChinaCloud, etc.)') ad_parser = subparsers.add_parser('ad', help='Connect via Active Directory') @@ -235,6 +254,13 @@ def __init__(self): help='AD group name or DN (repeatable). Prefix with "+" to select its direct subgroups.') ad_parser.add_argument('--ad-domain', dest='ad_domain', action='store', choices=['netbios', 'dns'], help='Use NetBIOS domain names (e.g., TEST) or DNS names (e.g., test.local)') + ad_parser.add_argument('--insecure', dest='insecure', action='store_true', + help='Disable TLS certificate validation for Active Directory (LDAPS) connection. ' + 'Insecure: do not use on untrusted networks.') + ad_parser.add_argument('--ad-ca-cert', dest='ca_cert', action='store', + help='Path to PEM file with CA certificate(s) to validate the Active Directory ' + '(LDAPS) server certificate against, when it is not issued by a CA already ' + 'trusted by this system.') for subparser in subparsers.choices.values(): subparser.exit = base.suppress_exit @@ -361,16 +387,32 @@ def execute(self, context: KeeperParams, **kwargs): scim_groups = None use_netbios_domain = ad_domain != 'dns' + insecure = kwargs.get('insecure') is True + ca_cert = kwargs.get('ca_cert') if not ad_url: raise base.CommandError('AD source requires AD URL') if os.name != 'nt' and not ad_user: raise base.CommandError('AD source requires AD User') + if ca_cert and not os.path.isfile(ca_cert): + raise base.CommandError(f'CA certificate file "{ca_cert}" was not found') + if insecure: + logging.warning('AD connect: TLS certificate validation is disabled (--insecure). ' + 'This connection is vulnerable to man-in-the-middle attacks.') + answer = base.user_choice( + bcolors.FAIL + bcolors.BOLD + '\nALERT!\n' + bcolors.ENDC + + '--insecure disables TLS certificate validation for the Active Directory (LDAPS) connection.\n' + 'This connection will be vulnerable to man-in-the-middle attacks. ' + 'Do not use this option on untrusted networks.' + + '\n\nDo you want to proceed?', 'yn', 'n') + if answer.lower() != 'y': + raise base.CommandError('Cancelled') try: if ad_user and not ad_password: ad_password = getpass.getpass(prompt=f'{ad_user} Password: ', stream=None) if not ad_password: raise base.CommandError('Cancelled') - data_source = AdCrmDataSource(ad_url, ad_user, ad_password, scim_groups, use_netbios_domain) + data_source = AdCrmDataSource(ad_url, ad_user, ad_password, scim_groups, use_netbios_domain, + insecure=insecure, ca_cert=ca_cert) ad_domains = data_source.resolve_domains() except Exception as e: raise base.CommandError(f'Error connecting to Active Directory: {e}') @@ -392,8 +434,6 @@ def execute(self, context: KeeperParams, **kwargs): azure_cloud = 'AzureChinaCloud' elif azure_cloud == 'GOV': azure_cloud = 'AzureUSGovernment' - elif azure_cloud == 'EU': - azure_cloud = 'AzureGermanCloud' else: azure_cloud = None else: @@ -789,13 +829,7 @@ def execute(self, context: KeeperParams, **kwargs) -> Optional[str]: linux = '' try: - hostname = whoami.get_hostname(context.rest_context.server_base) - for dc in constants.KEEPER_PUBLIC_HOSTS.values(): - if hostname.endswith(dc): - us = constants.KEEPER_PUBLIC_HOSTS['US'] - hostname = hostname[:-len(us)] + us - break - + hostname = constants.KEEPER_PUBLIC_HOSTS['US'] manifest_url = urlunparse(('https', hostname, '/pam/pedm/package-manifest.json', None, None, None)) rs = requests.get(manifest_url) manifest = rs.json() @@ -1667,23 +1701,51 @@ def execute(self, context: KeeperParams, **kwargs) -> Any: class PedmPolicyDeleteCommand(base.ArgparseCommand): def __init__(self): parser = argparse.ArgumentParser(prog='delete', description='Delete EPM policy') + parser.add_argument('-f', '--force', dest='force', action='store_true', + help='do not prompt for confirmation') parser.add_argument('policy', type=str, nargs='+', help='Policy UID or name') super().__init__(parser) def execute(self, context: KeeperParams, **kwargs) -> None: plugin = admin_plugin.get_pedm_plugin(context) - policies = PedmUtils.resolve_existing_policies(plugin, kwargs.get('policy')) - to_delete = [x.policy_uid for x in policies] + policy = kwargs.get('policy') + force = kwargs.get('force') is True + + missing_uids: Set[str] = set() + policies = PedmUtils.resolve_existing_policies(plugin, policy, missing_names=missing_uids) + to_delete = {x.policy_uid for x in policies} + if force and len(missing_uids) > 0: + for policy_uid in missing_uids: + try: + uid = utils.base64_url_decode(policy_uid) + if len(uid) == 16: + to_delete.add(policy_uid) + except: + pass + + if len(to_delete) == 0: + logging.info('No policies found') + return - rs = plugin.modify_policies(remove_policies=to_delete) + if not force: + answer = prompt_utils.user_choice(f'Do you want to remove {len(to_delete)} policy(s)?', 'yN', default='n') + if answer.lower() not in ('y', 'yes'): + return + + rs = plugin.modify_policies(remove_policies=list(to_delete)) if len(rs.remove) > 0: - status = rs.remove[0] - if isinstance(status, admin_types.EntityStatus) and not status.success: - raise base.CommandError(f'Failed to delete policy "{status.entity_uid}": {status.message}') + for status in rs.remove: + if isinstance(status, admin_types.EntityStatus) and not status.success: + message = f'Failed to delete policy "{status.entity_uid}": {status.message}' + if force: + logging.warning(message) + else: + raise base.CommandError(message) - policy_names = ', '.join(p.data.get('PolicyName') or p.policy_uid for p in policies) - logging.info('Successfully deleted policy: %s', policy_names) + if len(policies) > 0: + policy_names = ', '.join(p.data.get('PolicyName') or p.policy_uid for p in policies) + logging.info('Successfully deleted policy: %s', policy_names) class PedmPolicyAgentsCommand(base.ArgparseCommand): diff --git a/keepercommander/commands/scim.py b/keepercommander/commands/scim.py index 4a379fab7..6392bb1dd 100644 --- a/keepercommander/commands/scim.py +++ b/keepercommander/commands/scim.py @@ -68,6 +68,13 @@ help='Record UID with SCIM configuration') scim_push_parser.add_argument('--auto-approve', dest='auto_approve', action='store', choices=['on', 'off'], default='on', help='Auto approve SCIM teams') +scim_push_parser.add_argument('--insecure', dest='insecure', action='store_true', + help='Disable TLS certificate validation for Active Directory (LDAPS) connection. ' + 'Insecure: do not use on untrusted networks.') +scim_push_parser.add_argument('--ad-ca-cert', dest='ca_cert', action='store', + help='Path to PEM file with CA certificate(s) to validate the Active Directory ' + '(LDAPS) server certificate against, when it is not issued by a CA already ' + 'trusted by this system.') scim_push_parser.add_argument('target', help='SCIM ID') @@ -444,7 +451,23 @@ def verbose_logging(message): if not ad_password: raise CommandError('', f'Active Directory SCIM record "{record.title}" does not have "AD Password" field.\n' 'This field contains AD user password.') - data_source = AdCrmDataSource(ad_url, ad_user, ad_password, scim_groups, use_netbios_domain=False) + insecure = kwargs.get('insecure') is True + ca_cert = kwargs.get('ca_cert') + if ca_cert and not os.path.isfile(ca_cert): + raise CommandError('', f'CA certificate file "{ca_cert}" was not found') + if insecure: + logging.warning('AD connect: TLS certificate validation is disabled (--insecure). ' + 'This connection is vulnerable to man-in-the-middle attacks.') + answer = user_choice( + bcolors.FAIL + bcolors.BOLD + '\nALERT!\n' + bcolors.ENDC + + '--insecure disables TLS certificate validation for the Active Directory (LDAPS) connection.\n' + 'This connection will be vulnerable to man-in-the-middle attacks. ' + 'Do not use this option on untrusted networks.' + + '\n\nDo you want to proceed?', 'yn', 'n') + if answer.lower() != 'y': + raise CommandError('', 'Cancelled') + data_source = AdCrmDataSource(ad_url, ad_user, ad_password, scim_groups, use_netbios_domain=False, + insecure=insecure, ca_cert=ca_cert) elif source == 'record': json_data = record.notes if not json_data: diff --git a/keepercommander/commands/workflow/state_commands.py b/keepercommander/commands/workflow/state_commands.py index 492d36a75..95a99cae5 100644 --- a/keepercommander/commands/workflow/state_commands.py +++ b/keepercommander/commands/workflow/state_commands.py @@ -11,6 +11,7 @@ import argparse import json +import logging from datetime import datetime from ..base import Command, dump_report_data @@ -150,16 +151,59 @@ def execute(self, params: KeeperParams, **kwargs): print(f"\n{bcolors.WARNING}No active workflows{bcolors.ENDC}\n") return + checked_out_by = {} + lookup_failures = 0 + for wf in response.workflows: + flow_uid = bytes(wf.flowUid) + owner = wf.status.checkedOutBy or None + if not owner: + try: + owner = self._get_checked_out_by(params, wf) + except Exception: + lookup_failures += 1 + logging.debug( + 'Failed to resolve checkout owner for workflow %s', + utils.base64_url_encode(wf.flowUid), + exc_info=True, + ) + checked_out_by[flow_uid] = owner + + if lookup_failures: + logging.warning( + 'Checkout owner lookup failed for %d workflow(s); ' + 'the corresponding values may be blank.', + lookup_failures, + ) + if kwargs.get('format') == 'json': - self._print_json(params, response) + self._print_json(params, response, checked_out_by) else: - self._print_table(params, response) + self._print_table(params, response, checked_out_by) except Exception as e: raise CommandError('', f'Failed to get user access state: {sanitize_router_error(e)}') @staticmethod - def _print_json(params, response): + def _get_checked_out_by(params, workflow): + """Resolve checkout ownership from full state, which may be omitted by get_user_access_state.""" + if not workflow.flowUid: + return None + + state_query = workflow_pb2.WorkflowState() + state_query.flowUid = workflow.flowUid + state = _post_request_to_router( + params, 'get_workflow_state', + rq_proto=state_query, rs_type=workflow_pb2.WorkflowState, + ) + if not state or not state.status: + return None + + checked_out_by = state.status.checkedOutBy + return checked_out_by or None + + @staticmethod + def _print_json(params, response, checked_out_by=None): + checked_out_by = checked_out_by or {} result = { 'workflows': [ { @@ -169,7 +213,7 @@ def _print_json(params, response): 'stage': WorkflowFormatter.format_stage(wf.status.stage, wf.status), 'conditions': [WorkflowFormatter.format_conditions([c]) for c in wf.status.conditions], 'escalated': wf.status.escalated, - 'checked_out_by': wf.status.checkedOutBy or None, + 'checked_out_by': checked_out_by.get(bytes(wf.flowUid)), 'can_force_checkin': wf.status.canForceCheckIn, 'started_on': wf.status.startedOn or None, 'expires_on': wf.status.expiresOn or None, @@ -187,14 +231,15 @@ def _print_json(params, response): print(json.dumps(result, indent=2)) @staticmethod - def _print_table(params, response): + def _print_table(params, response, checked_out_by=None): + checked_out_by = checked_out_by or {} rows = [] for wf in response.workflows: stage = WorkflowFormatter.format_stage(wf.status.stage, wf.status) record_name = RecordResolver.resolve_name(params, wf.resource) record_uid = utils.base64_url_encode(wf.resource.value) if wf.resource.value else '' flow_uid = utils.base64_url_encode(wf.flowUid) if wf.flowUid else '' - checked_out_by = wf.status.checkedOutBy or '' + checkout_owner = checked_out_by.get(bytes(wf.flowUid)) or '' started = _fmt_ts_or_empty(wf.status.startedOn) expires = _fmt_ts_or_empty(wf.status.expiresOn) approved_by = '' @@ -204,7 +249,7 @@ def _print_table(params, response): for a in wf.status.approvedBy ] approved_by = '\n'.join(approved_names) - rows.append([stage, record_name, record_uid, flow_uid, checked_out_by, approved_by, started, expires]) + rows.append([stage, record_name, record_uid, flow_uid, checkout_owner, approved_by, started, expires]) headers = ['Stage', 'Record Name', 'Record UID', 'Flow UID', 'Checked Out By', 'Approved By', 'Started', 'Expires'] print() diff --git a/keepercommander/plugins/azureadpwd/azureadpwd.py b/keepercommander/plugins/azureadpwd/azureadpwd.py index c7375de94..e117adbe3 100644 --- a/keepercommander/plugins/azureadpwd/azureadpwd.py +++ b/keepercommander/plugins/azureadpwd/azureadpwd.py @@ -27,14 +27,12 @@ AZURE_GLOBAL_CLOUD = 'AzureCloud' AZURE_CHINA_CLOUD = 'AzureChinaCloud' AZURE_US_GOV_CLOUD = 'AzureUSGovernment' -AZURE_GERMAN_CLOUD = 'AzureGermanCloud' AZURE_CLOUD_URLS = { AZURE_GLOBAL_CLOUD: AzureEndpoint('https://login.microsoftonline.com', 'https://graph.microsoft.com'), AZURE_CHINA_CLOUD: AzureEndpoint('https://login.chinacloudapi.cn', 'https://microsoftgraph.chinacloudapi.cn'), AZURE_US_GOV_CLOUD: AzureEndpoint('https://login.microsoftonline.us', 'https://graph.microsoft.us'), - AZURE_GERMAN_CLOUD: AzureEndpoint('https://login.microsoftonline.de', 'https://graph.microsoft.de'), } # type: Dict[str, AzureEndpoint] @@ -53,8 +51,6 @@ def rotate(record, newpassword): # type: (vault.KeeperRecord, str) -> bool azure_cloud = AZURE_CHINA_CLOUD elif 'usgov' in azure_cloud: azure_cloud = AZURE_US_GOV_CLOUD - elif 'german' in azure_cloud: - azure_cloud = AZURE_GERMAN_CLOUD elif 'global' in azure_cloud: azure_cloud = AZURE_GLOBAL_CLOUD elif 'azurecloud' == azure_cloud: diff --git a/keepercommander/record_management.py b/keepercommander/record_management.py index e645a7cc8..93e1e8ca5 100644 --- a/keepercommander/record_management.py +++ b/keepercommander/record_management.py @@ -129,7 +129,7 @@ def add_record_to_folder(params, record, folder_uid=None, pb_only:bool=False): record_rs = next((x for x in rs.records if utils.base64_url_encode(x.record_uid) == record.record_uid), None) if record_rs: if record_rs.status != record_pb2.RS_SUCCESS: - raise KeeperApiError(record_rs.status, rs.message) + raise KeeperApiError(record_pb2.RecordModifyResult.Name(record_rs.status), record_rs.message) record.revision = rs.revision if record.linked_keys: for file_uid in record.linked_keys: diff --git a/keepercommander/scim/data_sources.py b/keepercommander/scim/data_sources.py index 3e48f5b96..b5966656e 100644 --- a/keepercommander/scim/data_sources.py +++ b/keepercommander/scim/data_sources.py @@ -43,7 +43,9 @@ def debug_logger(self, value): # type: (Callable[[str], None]) -> None class AdCrmDataSource(ICrmDataSource): - def __init__(self, ad_url, ad_user, ad_password, scim_groups, use_netbios_domain=False): # type: (str, Optional[str], Optional[str], Optional[List[str]], bool) -> None + def __init__(self, ad_url, ad_user, ad_password, scim_groups, + use_netbios_domain=False, insecure=False, ca_cert=None): + # type: (str, Optional[str], Optional[str], Optional[List[str]], bool, bool, Optional[str]) -> None super().__init__() if not ad_url.lower().startswith('ldap'): ad_url = 'ldaps://' + ad_url @@ -52,6 +54,8 @@ def __init__(self, ad_url, ad_user, ad_password, scim_groups, use_netbios_domain self.ad_password = ad_password if self.ad_user else None self.scim_groups = scim_groups or [] self.use_netbios_domain = use_netbios_domain + self.insecure = insecure + self.ca_cert = ca_cert self._domain_lookup = None # type: Optional[Dict[str, str]] @staticmethod @@ -81,14 +85,10 @@ def _direct_subgroups_filter(group_dn: str) -> str: @contextmanager def get_ldap_connection(self): ldap3 = AdCrmDataSource._get_ldap_module() - tls = None # type: Optional[ldap3.Tls] + tls = ldap3.Tls(validate=ssl.CERT_NONE if self.insecure else ssl.CERT_REQUIRED, ca_certs_file=self.ca_cert) if self.ad_user: + logging.debug('AD connect: LDAP Simple login') auth_method = ldap3.SIMPLE - if self.ad_url.startswith('ldap://'): - logging.debug('AD connect: Request TLS login for LDAP port') - tls = ldap3.Tls(validate=ssl.CERT_REQUIRED) - else: - logging.debug('AD connect: LDAPS Simple login') else: logging.debug('AD connect: Kerberos auth method. Requires Windows, domain user, and domain computer') auth_method = ldap3.SASL @@ -99,6 +99,9 @@ def get_ldap_connection(self): sasl_mechanism=ldap3.KERBEROS if auth_method == ldap3.SASL else None) as connection: try: connection.open() + if not server.ssl and auth_method == ldap3.SIMPLE: + if not connection.start_tls(): + raise Exception(f'Unable to start TLS on AD connection: {connection.last_error}') if not connection.bind(): raise Exception('Invalid AD username or password') yield connection @@ -643,7 +646,6 @@ def populate(self) -> Iterable[Union[ScimGroup, ScimUser]]: AZURE_GLOBAL_CLOUD: AZURE_ENDPOINT('https://login.microsoftonline.com', 'https://graph.microsoft.com'), 'AzureChinaCloud': AZURE_ENDPOINT('https://login.chinacloudapi.cn', 'https://microsoftgraph.chinacloudapi.cn'), 'AzureUSGovernment': AZURE_ENDPOINT('https://login.microsoftonline.us', 'https://graph.microsoft.us'), - 'AzureGermanCloud': AZURE_ENDPOINT('https://login.microsoftonline.de', 'https://graph.microsoft.de'), } @@ -835,8 +837,6 @@ def _resolve_cloud(azure_cloud): azure_cloud = 'AzureChinaCloud' elif 'usgov' in ac or 'us_gov' in ac or 'government' in ac: azure_cloud = 'AzureUSGovernment' - elif 'german' in ac or 'de' == ac: - azure_cloud = 'AzureGermanCloud' elif 'global' in ac or 'azurecloud' == ac or ac == 'azure': azure_cloud = AZURE_GLOBAL_CLOUD diff --git a/keepercommander/service/decorators/security.py b/keepercommander/service/decorators/security.py index 3d493f28a..d00a3d274 100644 --- a/keepercommander/service/decorators/security.py +++ b/keepercommander/service/decorators/security.py @@ -10,18 +10,24 @@ # import ipaddress +import os from functools import wraps from flask import request, jsonify from ..util.config_reader import ConfigReader +from ..config.config_validation import ConfigValidator, ValidationError from flask_limiter import Limiter from flask_limiter.util import get_remote_address from ..decorators.logging import logger +DEFAULT_RATE_LIMIT = "60/minute" + limiter = Limiter( key_func=get_remote_address, - default_limits=["60/minute", "600 per hour", "6000/day"], + default_limits=[DEFAULT_RATE_LIMIT, "600 per hour", "6000/day"], storage_uri="memory://" ) + +_cached_rate_limit = None def is_allowed_ip(ip_addr, allowed_ips_str, denied_ips_str): """Check if the given IP address is allowed based on allow/deny lists. @@ -67,10 +73,65 @@ def _ip_matches(parsed_ip, pattern): return parsed_ip == ipaddress.ip_address(pattern) except ValueError: return False - + + +def _validated_rate_limit(value, source): + """Validate and return rate limit, or None if invalid.""" + if not value: + return None + try: + return ConfigValidator.validate_rate_limit(value.strip()) + except ValidationError as e: + logger.warning(f"Invalid rate limit from {source} ({value!r}): {e}") + return None + + +def _resolve_rate_limit(): + """Resolve and validate rate limit once at startup/first call, caching the result. + + Priority order: + 1. KEEPER_RATE_LIMIT environment variable (for streamlined mode deployments) + 2. rate_limiting from config file + 3. Default: 60/minute + + Warns at most once per process if invalid values are detected. + """ + global _cached_rate_limit + if _cached_rate_limit is not None: + return _cached_rate_limit + + # Check environment variable first (highest priority for streamlined mode) + env_limit = os.environ.get('KEEPER_RATE_LIMIT') + if env_limit: + validated = _validated_rate_limit(env_limit, "KEEPER_RATE_LIMIT env var") + if validated: + logger.debug(f"Using rate limit from KEEPER_RATE_LIMIT env var: {validated}") + _cached_rate_limit = validated + return validated + # Fall through to config if env var is invalid (don't use hardcoded default) + + # Try reading from config file + config_limit = ConfigReader.read_config("rate_limiting") + if config_limit: + validated = _validated_rate_limit(config_limit, "config file") + if validated: + logger.debug(f"Using rate limit from config: {validated}") + _cached_rate_limit = validated + return validated + + # Empty config is normal default; log at DEBUG, not WARNING + logger.debug(f"Rate limit not configured. Using default {DEFAULT_RATE_LIMIT}.") + _cached_rate_limit = DEFAULT_RATE_LIMIT + return DEFAULT_RATE_LIMIT + + def get_rate_limit(): - """Get configured rate limit""" - return ConfigReader.read_config("rate_limiting") or "60/minute" + """Get cached rate limit for Flask-Limiter decorator. + + Called on every request, but uses cached value from _resolve_rate_limit() + to avoid re-validating and re-logging on every call. + """ + return _resolve_rate_limit() def get_rate_limit_key(): """Generate rate limit key per IP + endpoint for separate limits per endpoint""" diff --git a/keepercommander/service/util/parse_keeper_response.py b/keepercommander/service/util/parse_keeper_response.py index 9a226cc30..c05ed1415 100644 --- a/keepercommander/service/util/parse_keeper_response.py +++ b/keepercommander/service/util/parse_keeper_response.py @@ -95,15 +95,18 @@ def _find_parser_method(command: str) -> str: exact_patterns = { 'generate': '_parse_generate_command', 'ls': '_parse_ls_command', - 'tree': '_parse_tree_command', + 'tree': '_parse_tree_command', 'whoami': '_parse_whoami_command', 'this-device': '_parse_this_device_command', 'mkdir': '_parse_mkdir_command', 'record-add': '_parse_record_add_command', 'get': '_parse_get_command', 'download': '_parse_get_command', + 'nsf-mkdir': '_parse_nsf_mkdir_command', + 'nsf-rmdir': '_parse_nsf_rmdir_command', + 'nsf-rndir': '_parse_nsf_rndir_command', } - + for pattern, method_name in exact_patterns.items(): if command.startswith(pattern): return method_name @@ -115,12 +118,12 @@ def _find_parser_method(command: str) -> str: def parse_response(command: str, response: Any, log_output: str = None) -> Dict[str, Any]: """ Main parser that routes to specific command parsers based on the command type. - + Args: command (str): The executed command response (Any): Response from the keeper commander log_output (str, optional): Captured log output from command execution - + Returns: Dict[str, Any]: Structured JSON response """ @@ -133,11 +136,11 @@ def parse_response(command: str, response: Any, log_output: str = None) -> Dict[ } # Preprocess response once response_str, is_from_log = KeeperResponseParser._preprocess_response(response, log_output) - + # Handle completely empty responses if not response_str: return KeeperResponseParser._handle_empty_response(command) - + # Find the appropriate parser method (used for both log and non-log paths) parser_method_name = KeeperResponseParser._find_parser_method(command) @@ -146,18 +149,15 @@ def parse_response(command: str, response: Any, log_output: str = None) -> Dict[ command, response_str, None if is_from_log else log_output ) - # If from log output, use command-specific parser if available, else generic logging parser - if is_from_log: - return KeeperResponseParser._parse_logging_based_command(command, response_str) - - parser_method_name = KeeperResponseParser._find_parser_method(command) parser_method = getattr(KeeperResponseParser, parser_method_name) - + # Call the parser method with appropriate arguments - if parser_method_name in ['_parse_generate_command', '_parse_json_format_command', + if parser_method_name in ['_parse_generate_command', '_parse_json_format_command', '_parse_pam_project_import_command', '_parse_enterprise_push_command', '_parse_epm_policy_add_command']: return parser_method(command, response_str) + elif parser_method_name in ['_parse_nsf_mkdir_command', '_parse_nsf_rmdir_command', '_parse_nsf_rndir_command']: + return parser_method(response_str, parser_method_name.replace('_parse_', '').replace('_command', '')) else: return parser_method(response_str) if parser_method_name != '_parse_logging_based_command' else parser_method(command, response_str) @@ -571,6 +571,69 @@ def _parse_this_device_command(response: str) -> Dict[str, Any]: return result + @staticmethod + def _parse_nsf_mkdir_command(response: str, command_name: str) -> Dict[str, Any]: + """Parse nsf-mkdir command output.""" + return KeeperResponseParser._parse_nsf_folder_command(response, 'nsf-mkdir') + + @staticmethod + def _parse_nsf_rmdir_command(response: str, command_name: str) -> Dict[str, Any]: + """Parse nsf-rmdir command output.""" + return KeeperResponseParser._parse_nsf_folder_command(response, 'nsf-rmdir') + + @staticmethod + def _parse_nsf_rndir_command(response: str, command_name: str) -> Dict[str, Any]: + """Parse nsf-rndir command output.""" + return KeeperResponseParser._parse_nsf_folder_command(response, 'nsf-rndir') + + @staticmethod + def _parse_nsf_folder_command(response: str, command_name: str) -> Dict[str, Any]: + """Parse NSF folder commands (nsf-mkdir, nsf-rmdir, nsf-rndir) to extract folder UID. + + NSF commands return the folder UID directly from the API result, immune to concurrent log noise. + Extract UID directly and populate both data.folder_uid and top-level message field. + """ + response_str = response.strip() + lines = [ln.strip() for ln in response_str.split('\n') if ln.strip()] + + result = { + "status": "success", + "command": command_name, + "data": None + } + + for line in lines: + try: + data = json.loads(line) + if isinstance(data, dict) and 'folder_uid' in data: + folder_uid = data["folder_uid"] + result["data"] = { + "folder_uid": folder_uid, + "path": data.get("path"), + "name": data.get("name") + } + result["message"] = folder_uid + return result + except (json.JSONDecodeError, TypeError): + pass + + last_line = lines[-1] if lines else response_str + if re.match(r'^[a-zA-Z0-9_+/=-]+$', last_line): + result["data"] = { + "folder_uid": last_line + } + result["message"] = last_line + else: + uid_match = re.search(r'folder_uid=([a-zA-Z0-9_+/=-]+)', last_line) + if uid_match: + folder_uid = uid_match.group(1) + result["data"] = { + "folder_uid": folder_uid + } + result["message"] = folder_uid + + return result + @staticmethod def _parse_mkdir_command(response: str) -> Dict[str, Any]: """Parse 'mkdir' command output to extract folder UID, path, and name.""" @@ -587,26 +650,31 @@ def _parse_mkdir_command(response: str) -> Dict[str, Any]: try: data = json.loads(line) if isinstance(data, dict) and 'folder_uid' in data: + folder_uid = data["folder_uid"] result["data"] = { - "folder_uid": data["folder_uid"], + "folder_uid": folder_uid, "path": data.get("path"), "name": data.get("name") } + result["message"] = folder_uid return result except (json.JSONDecodeError, TypeError): pass last_line = lines[-1] if lines else response_str - if re.match(r'^[a-zA-Z0-9_-]+$', last_line): + if re.match(r'^[a-zA-Z0-9_+/=-]+$', last_line): result["data"] = { "folder_uid": last_line } + result["message"] = last_line else: - uid_match = re.search(r'folder_uid=([a-zA-Z0-9_-]+)', last_line) + uid_match = re.search(r'folder_uid=([a-zA-Z0-9_+/=-]+)', last_line) if uid_match: + folder_uid = uid_match.group(1) result["data"] = { - "folder_uid": uid_match.group(1) + "folder_uid": folder_uid } + result["message"] = folder_uid return result @@ -982,9 +1050,10 @@ def _handle_empty_response(command: str) -> Dict[str, Any]: """Handle commands that produce no output but are successful.""" # These commands truly produce no output (no logs, no stdout) and should be treated as silent success silent_success_commands = [ - "sync-down", "logout", "keep-alive", "set", "record-update", "append-notes", "mv", "ln" + "sync-down", "logout", "keep-alive", "set", "record-update", "append-notes", "mv", "ln", + "nsf-rmdir" ] - + if any(cmd in command for cmd in silent_success_commands): # Command-specific success messages for truly silent commands if "sync-down" in command: @@ -1003,9 +1072,11 @@ def _handle_empty_response(command: str) -> Dict[str, Any]: message = "Item moved successfully" elif "ln" in command: message = "Link created successfully" + elif "nsf-rmdir" in command: + message = "Folder removed successfully" else: message = "Command executed successfully" - + return { "status": "success", "command": command.split()[0] if command.split() else command, diff --git a/keepercommander/yubikey/yubikey.py b/keepercommander/yubikey/yubikey.py index 994c7e36d..b9c65ac7f 100644 --- a/keepercommander/yubikey/yubikey.py +++ b/keepercommander/yubikey/yubikey.py @@ -140,10 +140,10 @@ def yubikey_authenticate(request): # type: (dict) -> Optional[AuthenticationRes return options = request['publicKeyCredentialRequestOptions'].copy() - origin = '' + appid = '' if 'extensions' in options: extensions = options['extensions'] - origin = extensions.get('appid') or '' + appid = extensions.get('appid') or '' if 'largeBlob' not in options['extensions']: options['extensions']['largeBlob'] = {'read': None} @@ -156,6 +156,9 @@ def yubikey_authenticate(request): # type: (dict) -> Optional[AuthenticationRes if isinstance(challenge, str): options['challenge'] = utils.base64_url_decode(challenge) + # The server omits the appid extension when its host is local; the origin is then the RP ID's. + origin = appid or (f'https://{options["rpId"]}' if options.get('rpId') else '') + client = None # type: Optional[WebAuthnClient] data_collector = DefaultClientDataCollector(origin, verify=verify_rp_id_none) if os.name == 'nt': @@ -195,7 +198,7 @@ def yubikey_authenticate(request): # type: (dict) -> Optional[AuthenticationRes 'https://docs.keeper.io/enterprise-guide/two-factor-authentication#security-keys-fido-webauthn\n' 'Commander will use the fallback security key authentication method.\n\n' 'To use your Yubikey with Commander, please touch the flashing Security key one more time.\n') - options['rpId'] = origin + options['rpId'] = appid rq_options = PublicKeyCredentialRequestOptions.from_dict(options) rs = client.get_assertion(rq_options, event=evt) return rs.get_response(0) diff --git a/unit-tests/pam/test_workflow_my_access_checkout.py b/unit-tests/pam/test_workflow_my_access_checkout.py new file mode 100644 index 000000000..31a639905 --- /dev/null +++ b/unit-tests/pam/test_workflow_my_access_checkout.py @@ -0,0 +1,130 @@ +# _ __ +# | |/ /___ ___ _ __ ___ _ _ ® +# | '