From a8e169568b8d1de15d4e537442226412b237bda1 Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Tue, 25 Aug 2026 19:54:34 +0530 Subject: [PATCH 1/3] Mask secrets consistently in service-mode command logs (#2320) --- keepercommander/service/config/cli_handler.py | 4 +- keepercommander/service/core/request_queue.py | 10 +- .../service/decorators/api_logging.py | 39 ++-- keepercommander/service/decorators/logging.py | 80 +++++++- keepercommander/service/util/command_util.py | 12 +- .../service/util/request_validation.py | 4 +- unit-tests/service/test_log_sanitization.py | 189 ++++++++++++++++++ 7 files changed, 303 insertions(+), 35 deletions(-) create mode 100644 unit-tests/service/test_log_sanitization.py diff --git a/keepercommander/service/config/cli_handler.py b/keepercommander/service/config/cli_handler.py index ba3ecbb68..0a0f90889 100644 --- a/keepercommander/service/config/cli_handler.py +++ b/keepercommander/service/config/cli_handler.py @@ -14,7 +14,7 @@ import re from pathlib import Path from typing import Optional -from ..decorators.logging import logger, debug_decorator +from ..decorators.logging import logger, debug_decorator, sanitize_command_fields, sanitize_debug_data from ...params import KeeperParams class CommandHandler: @@ -32,7 +32,7 @@ def execute_cli_command(self, params: KeeperParams, command: str) -> str: cli.do_command(params, command) return output.getvalue() except Exception as e: - logger.debug(f"Error executing CLI command '{command}': {e}") + logger.debug(f"Error executing CLI command '{sanitize_command_fields(command)}': {sanitize_debug_data(str(e))}") return '' finally: sys.stdout = sys.__stdout__ diff --git a/keepercommander/service/core/request_queue.py b/keepercommander/service/core/request_queue.py index 3aed2816e..44e387e19 100644 --- a/keepercommander/service/core/request_queue.py +++ b/keepercommander/service/core/request_queue.py @@ -21,7 +21,7 @@ from dataclasses import dataclass, asdict from ..util.command_util import CommandExecutor -from ..decorators.logging import logger, debug_decorator +from ..decorators.logging import logger, debug_decorator, sanitize_command_fields, sanitize_debug_data def derive_owner_key(api_key: Optional[str]) -> Optional[str]: @@ -150,7 +150,7 @@ def submit_request(self, command: str, temp_files: list = None, self.request_queue.put(request, block=False) with self.data_lock: self.active_requests[request_id] = request - logger.info(f"Request {request_id} queued: {command}") + logger.info(f"Request {request_id} queued: {sanitize_command_fields(command)}") return request_id except queue.Full: logger.error("Error: Request queue is full") @@ -304,7 +304,7 @@ def _process_queue(self): self._cleanup_expired_requests() continue except Exception as e: - logger.error(f"Unexpected error in queue worker: {e}") + logger.error(f"Unexpected error in queue worker: {sanitize_debug_data(str(e))}") time.sleep(1) logger.info("Queue worker thread stopped") @@ -321,7 +321,7 @@ def _process_request(self, request: QueuedRequest): request.status = RequestStatus.PROCESSING request.started_at = datetime.now() - logger.info(f"Processing request {request.request_id}: {request.command}") + logger.info(f"Processing request {request.request_id}: {sanitize_command_fields(request.command)}") try: # Execute the command using existing CommandExecutor @@ -341,7 +341,7 @@ def _process_request(self, request: QueuedRequest): request.completed_at = datetime.now() request.error_message = str(e) - logger.error(f"Request {request.request_id} failed: {e}") + logger.error(f"Request {request.request_id} failed: {sanitize_debug_data(str(e))}") finally: # Clean up temporary files diff --git a/keepercommander/service/decorators/api_logging.py b/keepercommander/service/decorators/api_logging.py index b478e8ca0..d06ae84f2 100644 --- a/keepercommander/service/decorators/api_logging.py +++ b/keepercommander/service/decorators/api_logging.py @@ -14,8 +14,11 @@ from typing import Callable, Any from flask import request import time -import re -from .logging import logger +from .logging import logger, sanitize_command_fields, SENSITIVE_FIELD_TYPES + +# Legacy generic keys (kept for JSON payloads that aren't shaped like Keeper +# record fields, e.g. arbitrary nested config blobs). +_SENSITIVE_DICT_KEYS = frozenset({'password', 'login', 'secret', 'token', 'key'}) | SENSITIVE_FIELD_TYPES class SSLHandshakeFilter(logging.Filter): @@ -29,32 +32,44 @@ def filter(self, record): return True def sanitize_password_in_command(data): - """Sanitize password values in command string and filedata""" + """Sanitize password, login, secret and TOTP (oneTimeCode) values in command string and filedata""" if not data: return data - + sanitized = data.copy() - + # Sanitize command string if present - if 'command' in sanitized: - command = sanitized['command'] - # Pattern to match password=value (with or without quotes) - password_pattern = r"password=(['\"]?)([^'\"\s]{1,1024})\1" - sanitized['command'] = re.sub(password_pattern, r"password=\1***\1", command) - + if 'command' in sanitized and isinstance(sanitized['command'], str): + sanitized['command'] = sanitize_command_fields(sanitized['command']) + # Sanitize filedata if present if 'filedata' in sanitized: sanitized['filedata'] = _sanitize_nested_data(sanitized['filedata']) return sanitized +def _mask_field_value(value): + """Mask a Keeper record field's `value`, preserving its container shape.""" + if isinstance(value, list): + return ['***' for _ in value] + if isinstance(value, dict): + return {k: '***' for k in value} + return '***' + + def _sanitize_nested_data(data): """Recursively sanitize nested data structures""" if isinstance(data, dict): + field_type = data.get('type') + if isinstance(field_type, str) and field_type.lower() in SENSITIVE_FIELD_TYPES and 'value' in data: + sanitized = dict(data) + sanitized['value'] = _mask_field_value(data['value']) + return sanitized + sanitized = {} for key, value in data.items(): # Sanitize sensitive field names - if key.lower() in ['password', 'login', 'secret', 'token', 'key']: + if key.lower() in _SENSITIVE_DICT_KEYS: if isinstance(value, str) and len(value) > 0: sanitized[key] = '*' * min(len(value), 15) else: diff --git a/keepercommander/service/decorators/logging.py b/keepercommander/service/decorators/logging.py index da0bb0de5..7542aeb0f 100644 --- a/keepercommander/service/decorators/logging.py +++ b/keepercommander/service/decorators/logging.py @@ -14,9 +14,18 @@ import logging import sys, os, yaml import re +import shlex from enum import Enum from ... import utils +# Values that must never reach the logs when set via record-add/record-update/ +# nsf-record-* CLI args. +SENSITIVE_FIELD_TYPES = frozenset({ + 'password', 'login', 'secret', 'onetimecode', 'pincode', 'keypair', + 'privatekey', 'passphrase', 'paymentcard', 'bankaccount', + 'securityquestion', 'passkey', +}) + class LogLevel(Enum): ERROR = logging.ERROR WARNING = logging.WARNING @@ -112,15 +121,15 @@ def debug_decorator(fn: Callable) -> Callable: @wraps(fn) def wrapper(*args: Any, **kwargs: Any) -> Any: if logger._logger.isEnabledFor(logging.DEBUG): - args_repr = [repr(a) for a in args] - kwargs_repr = [f"{k}={v!r}" for k, v in kwargs.items()] + args_repr = [sanitize_debug_data(repr(a)) for a in args] + kwargs_repr = [f"{k}={sanitize_debug_data(repr(v))}" for k, v in kwargs.items()] signature = ", ".join(args_repr + kwargs_repr) logger.debug(f"Call: {fn.__name__}({signature})") - + value = fn(*args, **kwargs) - + if logger._logger.isEnabledFor(logging.INFO): - logger.debug(f"Return: {fn.__name__} → {value!r}") + logger.debug(f"Return: {fn.__name__} → {sanitize_debug_data(repr(value))}") return value return wrapper @@ -151,16 +160,71 @@ def sanitize_debug_data(data: str) -> str: (r'"secret"\s*:\s*"[^"]*"', '"secret": "***"'), (r'"token"\s*:\s*"[^"]*"', '"token": "***"'), (r'"key"\s*:\s*"[^"]*"', '"key": "***"'), - (r'password=[^\s]*', 'password=***'), - (r'login=[^\s]*', 'login=***'), + (r'\bpassword=[^\s]*', 'password=***'), + (r'\blogin=[^\s]*', 'login=***'), + # oneTimeCode=otpauth://totp/...?secret=... — mask the whole value, TOTP seed included + (r'\boneTimeCode=[^\s]*', 'oneTimeCode=***'), + (r'\bsecret=[^\s]*', 'secret=***'), + # Other sensitive record field types (see SENSITIVE_FIELD_TYPES) that can + # appear as bare CLI args on record-add/record-update/nsf-* commands. + (r'\bpinCode=[^\s]*', 'pinCode=***'), + (r'\bkeyPair=[^\s]*', 'keyPair=***'), + (r'\bprivateKey=[^\s]*', 'privateKey=***'), + (r'\bpassphrase=[^\s]*', 'passphrase=***'), + (r'\bpaymentCard=[^\s]*', 'paymentCard=***'), + (r'\bbankAccount=[^\s]*', 'bankAccount=***'), + (r'\bsecurityQuestion=[^\s]*', 'securityQuestion=***'), + (r'\bpasskey=[^\s]*', 'passkey=***'), # Sanitize email addresses in logs to protect PII (r'\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z|a-z]{2,}\b', '***@***.***'), ] for pattern, replacement in patterns: sanitized = re.sub(pattern, replacement, sanitized, flags=re.IGNORECASE) - + return sanitized +def _record_field_type(token_key: str) -> str: + """Extract the FIELD_TYPE from a record-add/record-update field token key. + + Field tokens follow [f.|c.][.]= (see the + `record-add`/`record-update` --syntax-help). Custom fields carrying a + sensitive type (e.g. c.secret.APIKey=...) must be masked the same as bare + fields (secret=...). + """ + key = token_key[2:] if token_key[:2] in ('f.', 'c.') else token_key + return key.split('.', 1)[0] + + +def sanitize_command_fields(command: str) -> str: + """Mask sensitive record field values (password, secret, keyPair, private + key passphrases, payment/bank data, security answers, ...) in a + record-add/record-update/nsf-record-* command string. + + Unlike the plain keyword patterns in `sanitize_debug_data`, this masks + labeled and custom fields too (password.Label=..., c.secret.APIKey=...), + which a literal `password=` substring match cannot catch. + """ + if not command: + return command + + try: + tokens = shlex.split(command, posix=True) + except ValueError: + # Unbalanced quotes: fall back to whitespace split so we still mask + # what we can instead of logging the raw string. + tokens = command.split() + + masked_tokens = [] + for token in tokens: + key, sep, value = token.partition('=') + if sep and value and _record_field_type(key).lower() in SENSITIVE_FIELD_TYPES: + masked_tokens.append(f'{key}=***') + else: + masked_tokens.append(token) + + return sanitize_debug_data(' '.join(masked_tokens)) + + logger = GlobalLogger() \ No newline at end of file diff --git a/keepercommander/service/util/command_util.py b/keepercommander/service/util/command_util.py index 2a70d8a96..cb7d94bf6 100644 --- a/keepercommander/service/util/command_util.py +++ b/keepercommander/service/util/command_util.py @@ -27,7 +27,7 @@ ) from .verified_command import Verifycommand from ..core.globals import get_current_params -from ..decorators.logging import logger, debug_decorator, sanitize_debug_data +from ..decorators.logging import logger, debug_decorator, sanitize_debug_data, sanitize_command_fields from ... import cli, utils from ...crypto import encrypt_aes_v2 from ...error import KeeperApiError @@ -149,7 +149,7 @@ def _finalize_parsed_response(cls, response: Any) -> Tuple[Any, int]: @classmethod def execute(cls, command: str) -> Tuple[Any, int]: - logger.debug(f"Executing command: {command}") + logger.debug(f"Executing command: {sanitize_command_fields(command)}") validation_error = cls.validate_command(command) if validation_error: @@ -209,7 +209,7 @@ def execute(cls, command: str) -> Tuple[Any, int]: try: SailPointService.after_command(params, command, success=True) except Exception as e: - logger.error(f'SailPoint post-process failed: {e}') + logger.error(f'SailPoint post-process failed: {sanitize_debug_data(str(e))}') err = { 'status': 'error', 'error': ( @@ -224,18 +224,18 @@ def execute(cls, command: str) -> Tuple[Any, int]: return response, status_code except CommandExecutionError as e: # Return the actual command error instead of generic "server busy" - logger.error(f"Command execution error: {e}") + logger.error(f"Command execution error: {sanitize_debug_data(str(e))}") if is_throttle_error(e): return throttle_error_response(str(e)) return {"status": "error", "error": str(e)}, 400 except KeeperApiError as e: if is_throttle_error(e): return throttle_error_response(e.message or str(e), e.result_code) - logger.error(f"Unexpected error during command execution: {e}") + logger.error(f"Unexpected error during command execution: {sanitize_debug_data(str(e))}") return {"status": "error", "error": f"Unexpected error: {str(e)}"}, 500 except Exception as e: if is_throttle_error(e): return throttle_error_response(str(e)) # Log unexpected errors and return a proper error response - logger.error(f"Unexpected error during command execution: {e}") + logger.error(f"Unexpected error during command execution: {sanitize_debug_data(str(e))}") return {"status": "error", "error": f"Unexpected error: {str(e)}"}, 500 diff --git a/keepercommander/service/util/request_validation.py b/keepercommander/service/util/request_validation.py index 5b3019a0d..cff1051f0 100644 --- a/keepercommander/service/util/request_validation.py +++ b/keepercommander/service/util/request_validation.py @@ -15,7 +15,7 @@ import tempfile import os import json -from ..decorators.logging import logger +from ..decorators.logging import logger, sanitize_command_fields class RequestValidator: @@ -46,7 +46,7 @@ def validate_and_escape_command(request_data: Dict[str, Any]) -> Tuple[Optional[ # Escape HTML to prevent XSS escaped_command = escape(command) - logger.debug(f"Command validated and escaped: {escaped_command}") + logger.debug(f"Command validated and escaped: {sanitize_command_fields(escaped_command)}") return escaped_command, None @staticmethod diff --git a/unit-tests/service/test_log_sanitization.py b/unit-tests/service/test_log_sanitization.py new file mode 100644 index 000000000..681fa3add --- /dev/null +++ b/unit-tests/service/test_log_sanitization.py @@ -0,0 +1,189 @@ +from unittest import TestCase, mock + +from keepercommander.service.decorators.logging import ( + SENSITIVE_FIELD_TYPES, + sanitize_command_fields, + sanitize_debug_data, +) +from keepercommander.service.decorators.api_logging import ( + _sanitize_nested_data, + sanitize_password_in_command, +) +from keepercommander.service.util.command_util import CommandExecutor +from keepercommander.service.util.exceptions import CommandExecutionError +from keepercommander.service.util.request_validation import RequestValidator +from keepercommander.service.config.cli_handler import CommandHandler + +SECRET_VALUE = "TopSecretValue123" + + +class TestSanitizeCommandFields(TestCase): + """Table-driven coverage: every SENSITIVE_FIELD_TYPES entry must be masked + whether it's a bare field, a labeled field, a custom (c.) field, or a + field/custom (f./c.) field with a dotted label.""" + + def _assert_masked(self, command): + result = sanitize_command_fields(command) + self.assertNotIn(SECRET_VALUE, result) + self.assertIn('***', result) + + def test_bare_field(self): + for field_type in sorted(SENSITIVE_FIELD_TYPES): + with self.subTest(field_type=field_type): + self._assert_masked(f'record-add -rt login -t x {field_type}={SECRET_VALUE} --force') + + def test_labeled_field(self): + for field_type in sorted(SENSITIVE_FIELD_TYPES): + with self.subTest(field_type=field_type): + self._assert_masked( + f'record-update --uid=XYZ f.{field_type}.MyLabel={SECRET_VALUE} --force' + ) + + def test_custom_section_field(self): + for field_type in sorted(SENSITIVE_FIELD_TYPES): + with self.subTest(field_type=field_type): + self._assert_masked( + f'record-update --uid=XYZ c.{field_type}.CustomLabel={SECRET_VALUE} --force' + ) + + def test_dotted_label_without_section_prefix(self): + for field_type in sorted(SENSITIVE_FIELD_TYPES): + with self.subTest(field_type=field_type): + self._assert_masked(f'record-add -rt general -t x {field_type}.SubLabel={SECRET_VALUE} --force') + + def test_non_sensitive_field_untouched(self): + # A field merely labeled with a sensitive-looking substring (not a + # sensitive field TYPE) must not be masked or corrupted. + result = sanitize_command_fields('record-add -rt login -t x c.text.MyLoginId=notasecret --force') + self.assertIn('c.text.MyLoginId=notasecret', result) + + def test_label_containing_keyword_as_substring_not_corrupted(self): + # Regression: password=[^\s]* without a word boundary used to eat into + # "OldPassword", turning the label into "Oldpassword". + result = sanitize_command_fields(f'record-update --uid=XYZ f.password.OldPassword={SECRET_VALUE} --force') + self.assertIn('f.password.OldPassword=***', result) + + def test_quoted_value_with_spaces_fully_masked(self): + result = sanitize_command_fields('record-add -rt bankCard -t x paymentCard="4111111111111111 04/2026 123" --force') + self.assertNotIn('4111111111111111', result) + self.assertNotIn('123', result) + + def test_malformed_quoting_falls_back_and_still_masks(self): + result = sanitize_command_fields(f'record-add password="{SECRET_VALUE}') + self.assertNotIn(SECRET_VALUE, result) + + +class TestFiledataSanitization(TestCase): + """Keeper record field JSON shape: {"type": "password", "value": [...]}. + The secret lives under `value`, not under a sensitively-named dict key.""" + + def test_sensitive_type_value_shape_is_masked(self): + filedata = [ + {"type": "login", "value": ["bob"]}, + {"type": "password", "value": [SECRET_VALUE]}, + {"type": "keyPair", "value": [{"publicKey": "pub", "privateKey": SECRET_VALUE}]}, + {"type": "paymentCard", "value": [{"cardNumber": "4111111111111111", + "cardExpirationDate": "04/2026", + "cardSecurityCode": "123"}]}, + ] + sanitized = _sanitize_nested_data(filedata) + dumped = str(sanitized) + self.assertNotIn(SECRET_VALUE, dumped) + self.assertNotIn('4111111111111111', dumped) + self.assertNotIn('123', dumped) + + def test_non_sensitive_type_value_untouched(self): + filedata = [{"type": "text", "value": ["not a secret"]}] + sanitized = _sanitize_nested_data(filedata) + self.assertEqual(sanitized, filedata) + + def test_sanitize_password_in_command_masks_command_and_filedata(self): + payload = { + "command": f'record-add -rt login -t x password={SECRET_VALUE} --force', + "filedata": [{"type": "password", "value": [SECRET_VALUE]}], + } + sanitized = sanitize_password_in_command(payload) + dumped = str(sanitized) + self.assertNotIn(SECRET_VALUE, dumped) + + +class TestRequestValidationLogging(TestCase): + """Regression: validate_and_escape_command must not log the raw command.""" + + def test_debug_log_is_masked(self): + command = f'record-add -rt login -t x password={SECRET_VALUE} --force' + with mock.patch('keepercommander.service.util.request_validation.logger.debug') as mock_debug: + escaped_command, error = RequestValidator.validate_and_escape_command({"command": command}) + + self.assertIsNone(error) + # The value returned for execution must remain intact... + self.assertIn(SECRET_VALUE, escaped_command) + # ...but nothing logged should contain it. + mock_debug.assert_called_once() + self.assertNotIn(SECRET_VALUE, mock_debug.call_args[0][0]) + + +class TestCliHandlerLogging(TestCase): + """Regression: execute_cli_command must not log the raw command or a raw exception message.""" + + def test_error_path_masks_command_and_exception(self): + command = f'record-add -rt login -t x password={SECRET_VALUE} --force' + handler = CommandHandler() + params = mock.Mock(service_mode=False) + + with mock.patch('keepercommander.cli.do_command', + side_effect=Exception(f"boom while handling password={SECRET_VALUE}")), \ + mock.patch('keepercommander.service.config.cli_handler.logger.debug') as mock_debug: + result = handler.execute_cli_command(params, command) + + self.assertEqual(result, '') + # debug_decorator logs a "Call:" line and CommandHandler logs the error; + # none of them should contain the secret. + for call in mock_debug.call_args_list: + self.assertNotIn(SECRET_VALUE, call[0][0]) + + +class TestCommandExecutorErrorLogging(TestCase): + """Regression: CommandExecutor.execute must not leak secrets via str(exception).""" + + def test_command_execution_error_is_sanitized_in_logs(self): + command = f'record-add -rt login -t x password={SECRET_VALUE} --force' + + with mock.patch('keepercommander.service.core.globals.ensure_params_loaded', + return_value=mock.Mock(service_mode=False)), \ + mock.patch('keepercommander.service.util.command_util.Verifycommand.validate_service_mode_restrictions', + return_value=None), \ + mock.patch('keepercommander.service.util.command_util.Verifycommand.validate_enterprise_user_add_role_force', + return_value=None), \ + mock.patch.object(CommandExecutor, 'capture_output_and_logs', + side_effect=CommandExecutionError(f"failed on password={SECRET_VALUE}")), \ + mock.patch('keepercommander.service.util.command_util.logger.error') as mock_error: + response, status_code = CommandExecutor.execute(command) + + self.assertEqual(status_code, 400) + mock_error.assert_called_once() + self.assertNotIn(SECRET_VALUE, mock_error.call_args[0][0]) + + +class TestRequestQueueErrorLogging(TestCase): + """Regression: the queue worker's failure log must not leak secrets from str(exception).""" + + def test_process_request_failure_is_sanitized_in_logs(self): + from datetime import datetime + from keepercommander.service.core.request_queue import QueuedRequest, RequestStatus, queue_manager + + request = QueuedRequest( + request_id='test-request-id', + command=f'record-add -rt login -t x password={SECRET_VALUE} --force', + status=RequestStatus.PROCESSING, + created_at=datetime.now(), + ) + + with mock.patch.object(CommandExecutor, 'execute', + side_effect=Exception(f"boom password={SECRET_VALUE}")), \ + mock.patch('keepercommander.service.core.request_queue.logger.error') as mock_error, \ + mock.patch('keepercommander.service.core.request_queue.logger.info'): + queue_manager._process_request(request) + + mock_error.assert_called_once() + self.assertNotIn(SECRET_VALUE, mock_error.call_args[0][0]) From 1386f0969863281788e35693074c059850bdc327 Mon Sep 17 00:00:00 2001 From: Sergey Kolupaev Date: Tue, 25 Aug 2026 07:29:45 -0700 Subject: [PATCH 2/3] Release 18.1.3 --- keepercommander/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keepercommander/__init__.py b/keepercommander/__init__.py index 090609d3e..08e4620b5 100644 --- a/keepercommander/__init__.py +++ b/keepercommander/__init__.py @@ -10,4 +10,4 @@ # Contact: commander@keepersecurity.com # -__version__ = '18.1.2' +__version__ = '18.1.3' From 18cf3ef07f12ec0068f564dd17f4c0a41d74f7ac Mon Sep 17 00:00:00 2001 From: pvagare-ks Date: Mon, 24 Aug 2026 19:00:20 +0530 Subject: [PATCH 3/3] Added --nsf support for cyberark, cyberark_portal imports --- docs/cyberark-pam-import.md | 5 +- keepercommander/importer/commands.py | 12 +- keepercommander/importer/cyberark/cyberark.py | 103 +++--- .../importer/cyberark/pam/client.py | 303 +++++++++++++++--- .../importer/cyberark/pam/constants.py | 3 + .../cyberark_portal/cyberark_portal.py | 55 +++- keepercommander/importer/imp_exp.py | 181 ++++++++++- keepercommander/importer/importer.py | 2 + keepercommander/importer/nsf_import.py | 42 ++- tests/test_cyberark_pam_import.py | 156 +++++++++ unit-tests/test_nsf_import.py | 90 ++++++ 11 files changed, 812 insertions(+), 140 deletions(-) diff --git a/docs/cyberark-pam-import.md b/docs/cyberark-pam-import.md index da5ee70d7..a95b16e00 100644 --- a/docs/cyberark-pam-import.md +++ b/docs/cyberark-pam-import.md @@ -132,6 +132,8 @@ Every pamUser nested in a resource gets: ### Privilege Cloud (*.cyberark.cloud) - OAuth2 service account via `POST /oauth2/platformtoken` +- Interactive user login (password + MFA) via CyberArk Identity `StartAuthentication` / `AdvanceAuthentication` +- SSO / external IdP via native-client `OobIdPAuth` (browser redirect + `/Security/OobAuthStatus` poll, or OOBAUTHPIN) - Tenant ID discovery via `platform-discovery.cyberark.cloud` - Tenant formats: `abc1234`, `mycompany`, `abc1234.id`, `tenant.my.idaptive.app`, full URL - URL rewrite: `tenant.cyberark.cloud` → `tenant.privilegecloud.cyberark.cloud` @@ -143,6 +145,7 @@ Every pamUser nested in a resource gets: | KEEPER_CYBERARK_ID_TENANT | Identity tenant ID (Privilege Cloud) | | KEEPER_CYBERARK_USERNAME | Username or service account client ID | | KEEPER_CYBERARK_PASSWORD | Password or client secret | +| KEEPER_CYBERARK_AUTH_METHOD | `service` or `interactive` / `sso` (Privilege Cloud) | | KEEPER_CYBERARK_LOGON_TYPE | Logon type for self-hosted (CyberArk/LDAP/RADIUS/Windows) | | KEEPER_CYBERARK_SAFES | Comma-separated safe names | | KEEPER_CYBERARK_SAFES_PATH | Path to safes.txt file | @@ -256,7 +259,7 @@ Before building the import JSON, the importer warns about: |---|---|---| | Self-hosted PVWA (v10.4+) | CyberArk/LDAP/RADIUS/Windows | Implemented | | Privilege Cloud (SaaS) | OAuth2 service account | Implemented | -| Privilege Cloud Shared Services (ISPSS) | OAuth2 with platform discovery | Implemented | +| Privilege Cloud Shared Services (ISPSS) | OAuth2 / interactive MFA / SSO (OobIdPAuth) | Implemented | | User Portal (Identity) | Separate importer (cyberark_portal) | Different scope | --- diff --git a/keepercommander/importer/commands.py b/keepercommander/importer/commands.py index 96ef92fed..32a54f7f5 100644 --- a/keepercommander/importer/commands.py +++ b/keepercommander/importer/commands.py @@ -62,7 +62,8 @@ def register_command_info(aliases, command_info): import_parser.add_argument('-s', '--shared', dest='shared', action='store_true', help='import folders as Keeper shared folders') import_parser.add_argument('--nsf', dest='use_nsf', action='store_true', - help='import folders and records into Nested Share Folders (works with any --format)') + help='import folders and records into Nested Share Folders ' + '(json, csv, keepass, cyberark, cyberark_portal, …)') 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', @@ -82,7 +83,7 @@ def register_command_info(aliases, command_info): import_parser.add_argument('--file-cache', dest='tmpdir', action='store', help='temp directory used to cache encrypted attachment imports') import_parser.add_argument('--show-skipped', dest='show_skipped', action='store_true', - help='Display skipped records') + help='display skipped records (always enabled for CyberArk imports)') import_parser.add_argument('--secret-ids', dest='secret_ids', action='store', help='Comma separated list of secret IDs to fetch (Thycotic)') import_parser.add_argument('--target-node', '--node', dest='target_node', action='store', @@ -173,6 +174,8 @@ def register_command_info(aliases, command_info): Nested Share Folders (NSF): import --format=csv --nsf sample_data/import_nsf.csv +import --format=cyberark --nsf https://tenant.privilegecloud.cyberark.cloud +import --format=cyberark_portal --nsf https://tenant.id.cyberark.cloud ''' json_instructions = '''JSON Import Instructions @@ -186,8 +189,11 @@ def register_command_info(aliases, command_info): To load the sample file into your vault, run this command: import --format=json sample_data/import.json.txt -Nested Share Folders (NSF) — works with any --format (json, csv, keepass, …): +Nested Share Folders (NSF) — works with any --format (json, csv, keepass, +cyberark, cyberark_portal, …): import --format=json --nsf sample_data/import_nsf.txt +import --format=cyberark --nsf https://tenant.privilegecloud.cyberark.cloud +import --format=cyberark_portal --nsf https://tenant.id.cyberark.cloud With --nsf, shared_folders[].permissions from JSON are granted on Nested Share Folders after folders and records are created. Prefer NSF roles: diff --git a/keepercommander/importer/cyberark/cyberark.py b/keepercommander/importer/cyberark/cyberark.py index cf26e44f0..c66a2bd9f 100644 --- a/keepercommander/importer/cyberark/cyberark.py +++ b/keepercommander/importer/cyberark/cyberark.py @@ -27,6 +27,7 @@ from ..importer import ( BaseDownloadMembership, BaseImporter, + Folder, Permission, Record, RecordField, @@ -260,6 +261,8 @@ def map_member(member): class CyberArkImporter(BaseImporter): + verbose_import_summary = True + # Delay between requests to avoid hitting the API rate limits DELAY = 0.025 # CyberArk REST API endpoints (relative to the base URL) @@ -735,63 +738,20 @@ def _authenticate_pvwa(self, filename): if not self._maybe_configure_client_cert(pvwa_host): return None if pvwa_host.endswith(".cyberark.cloud"): - pvwa_host = f"{pvwa_host.split('.')[0]}.privilegecloud.cyberark.cloud" - self._verify_tls = True - id_tenant = environ.get("_CYBERARK_ID_TENANT") or prompt("CyberArk Identity Tenant ID: ") - if re.match(r"^[A-Za-z]{3}\d{4}$", id_tenant): - id_tenant += ".id" - client_id = environ.get("_CYBERARK_USERNAME") or prompt("CyberArk service user name: ") - client_secret = environ.get("_CYBERARK_PASSWORD") or prompt( - "CyberArk service user password: ", is_password=True - ) - token_url = f"https://{id_tenant}.cyberark.cloud/oauth2/platformtoken" + from .pam.client import CyberArkPVWAClient + try: - response = self._request( - "POST", - token_url, - data={ - "grant_type": "client_credentials", - "client_id": client_id, - "client_secret": client_secret, - }, - timeout=self.TIMEOUT, - ) - except requests.exceptions.ConnectionError as e: - print_formatted_text( - HTML( - "OAuth2 authorization token request failed: " - f"could not connect to {id_tenant}.cyberark.cloud.\n" - "Verify the CyberArk Identity Tenant ID is correct (check the CyberArk Identity " - "Admin Portal URL — the first label of the hostname is your tenant ID) and that " - "your machine has network/DNS access to it." - ) - ) - print_formatted_text(HTML(f"Details: {e}")) + client = CyberArkPVWAClient(filename) + except ValueError as exc: + print_formatted_text(HTML(f"{exc}")) return None - except requests.exceptions.RequestException as e: - print_formatted_text( - HTML(f"OAuth2 authorization token request failed: {e}") - ) + if not client.authenticate(): return None - if response.status_code != 200: - print_formatted_text( - HTML( - f"OAuth2 authorization token request failed with status code {response.status_code}" - ) - ) - try: - print_formatted_text(HTML(f"Response: {response.text[:500]}")) - except Exception: - pass - return None - try: - access_token = response.json()["access_token"] - except (ValueError, KeyError) as e: - print_formatted_text( - HTML(f"OAuth2 response did not contain an access_token: {e}") - ) - return None - authorization_token = f"Bearer {access_token}" + pvwa_host = client.pvwa_host + if client.query_params: + query_params.update(client.query_params) + authorization_token = client.auth_token + self._verify_tls = True else: login_type = environ.get("_CYBERARK_LOGON_TYPE") or prompt( "CyberArk logon type (Cyberark, LDAP, RADIUS or Windows): " @@ -825,7 +785,7 @@ def _authenticate_pvwa(self, filename): ) return None authorization_token = response.text.strip('"') - print_formatted_text(HTML("Log on successful")) + print_formatted_text(HTML("Log on successful")) return pvwa_host, authorization_token, query_params def _resolve_safes(self, pvwa_host, authorization_token): @@ -979,6 +939,7 @@ def _do_import_inner(self, filename, **kwargs): if auth is None: return pvwa_host, authorization_token, query_params = auth + use_nsf = bool(kwargs.get("use_nsf")) params = kwargs.get("params") will_teams = environ.get("_CYBERARK_SKIP_TEAMS", "").lower() not in ("1", "true", "yes") @@ -994,7 +955,14 @@ def _do_import_inner(self, filename, **kwargs): if not safes: return - + if use_nsf: + print_formatted_text( + HTML( + "\nNSF mode: CyberArk safes will be created as " + "Nested Share Folders and accounts as NSF records." + ) + ) + print_formatted_text(HTML("\nScanning CyberArk safes for accounts to migrate...")) safe_accounts = {} for safe in safes: @@ -1093,6 +1061,10 @@ def _do_import_inner(self, filename, **kwargs): f"\nYou are about to import data from CyberArk PVWA {pvwa_host} into Keeper:", f" - {total_accounts} account(s) across {len(safe_accounts)} safe(s) as Keeper records", ] + if use_nsf: + summary_lines.append( + " - Safes as Nested Share Folders (--nsf); records created with the NSF API" + ) if group_names: summary_lines.append(f" - {len(group_names)} user group(s) as Keeper teams and roles") if eligible_users: @@ -1123,12 +1095,23 @@ def _do_import_inner(self, filename, **kwargs): tabulate([{"ID": x["id"], "Safe": x["safeName"], "Account": x["name"]} for x in accounts], headers="keys"), end="\n\n", ) + if use_nsf: + # Explicit NSF folder so prepare_nsf_folders has a SharedFolder target; + # record folder paths also resolve to the same NSF node. + nsf_folder = SharedFolder() + nsf_folder.path = safe.replace(PathDelimiter, 2 * PathDelimiter) + yield nsf_folder with _suppress_progressbar_executor_noise(), ProgressBar() as pb: skip_all = {} skipped_accounts = [] for r in pb(accounts, total=len(accounts)): - folder = SharedFolder() - folder.domain = r["safeName"] + folder = Folder() + # Classic import places each safe as a shared-folder domain. + # --nsf uses a path so Nested Share Folders are created instead. + if use_nsf: + folder.path = r["safeName"].replace(PathDelimiter, 2 * PathDelimiter) + else: + folder.domain = r["safeName"].replace(PathDelimiter, 2 * PathDelimiter) record = Record() record.folders = [folder] record.title = re.sub(rf"^.*{re.escape(r['platformId'])}[\-_ ]", "", r["name"]) @@ -1155,7 +1138,7 @@ def _do_import_inner(self, filename, **kwargs): "Authorization": authorization_token, "Content-Type": "application/json", }, - json={"reason": "Keeper Commander Import"}, + json={"reason": "test"}, timeout=self.TIMEOUT, verify=True if pvwa_host.endswith(".cyberark.cloud") else self._verify_tls, cert=None if pvwa_host.endswith(".cyberark.cloud") else self._client_cert, @@ -2249,7 +2232,7 @@ def download_membership(self, params, **kwargs): shared_folder = SharedFolder() shared_folder.uid = str(safe.get("id") or safe_url_id) - shared_folder.path = safe_name + shared_folder.path = safe_name.replace(PathDelimiter, 2 * PathDelimiter) shared_folder.permissions = [] skipped_service = 0 diff --git a/keepercommander/importer/cyberark/pam/client.py b/keepercommander/importer/cyberark/pam/client.py index e8c982834..4f85a73dc 100644 --- a/keepercommander/importer/cyberark/pam/client.py +++ b/keepercommander/importer/cyberark/pam/client.py @@ -16,13 +16,19 @@ import math import re import sys +import webbrowser from os import environ, path from typing import Any, Dict, List, Optional, Tuple from urllib.parse import parse_qsl, quote, unquote, urljoin, urlparse import requests as _requests_module -from .constants import MAX_FETCH_RECORDS, VALID_LOGON_TYPES, IDENTITY_LOGIN_SUCCESS +from .constants import ( + MAX_FETCH_RECORDS, + VALID_LOGON_TYPES, + IDENTITY_LOGIN_SUCCESS, + IDENTITY_IDP_STATE_SUCCESS, +) from .ui import _esc @@ -67,6 +73,9 @@ class CyberArkPVWAClient: # CyberArk Identity out-of-band (push / SMS / email) MFA polling settings. IDENTITY_OOB_POLL_INTERVAL = 2 # seconds between push-notification polls IDENTITY_OOB_POLL_TIMEOUT = 120 # give up on an unanswered push after this many seconds + # CyberArk Identity SSO / external IdP (OobIdPAuth) polling settings. + IDENTITY_IDP_POLL_INTERVAL = 2 # seconds between /Security/OobAuthStatus polls + IDENTITY_IDP_POLL_TIMEOUT = 360 # give up waiting for browser SSO after this many seconds ENDPOINTS = { "accounts": "Accounts", "account_password": "Accounts/{account_id}/Password/Retrieve", @@ -237,12 +246,15 @@ def _auth_privilege_cloud(self) -> bool: Two methods are supported: 1. Service account — OAuth2 ``client_credentials`` against ``/oauth2/platformtoken`` (non-interactive, no MFA). - 2. Interactive user login with MFA / 2FA via the CyberArk Identity - ``StartAuthentication`` / ``AdvanceAuthentication`` flow. + 2. Interactive user login via CyberArk Identity + ``StartAuthentication`` / ``AdvanceAuthentication``, including + password + MFA and SSO / external IdP (``OobIdPAuth`` browser + redirect with ``/Security/OobAuthStatus`` polling or PIN). Both produce a Bearer token that the Privilege Cloud REST API accepts. The method is selected via the ``KEEPER_CYBERARK_AUTH_METHOD`` env var - (``service`` / ``interactive``) or an interactive prompt. + (``service`` / ``interactive``), supplied service credentials, or the + availability of an interactive terminal. """ id_host = self._resolve_identity_host() if not id_host: @@ -251,6 +263,10 @@ def _auth_privilege_cloud(self) -> bool: return self._auth_privilege_cloud_interactive(id_host) return self._auth_privilege_cloud_service(id_host) + def _privilege_cloud_tenant_name(self) -> str: + """Return the Privilege Cloud tenant id used by StartAuthentication.""" + return self.pvwa_host.split(".")[0] + def _resolve_identity_host(self) -> Optional[str]: """Resolve the CyberArk Identity host for the configured tenant. @@ -260,8 +276,38 @@ def _resolve_identity_host(self) -> Optional[str]: - 'abc1234.id' → abc1234.id.cyberark.cloud (already qualified) - 'https://...' → extracted hostname used directly - 'tenant.my.idaptive.app' → tenant.my.idaptive.app (legacy Idaptive) + + When no tenant override env var is set, the tenant subdomain is taken + from the PVWA hostname (e.g. ``metron.privilegecloud.cyberark.cloud`` + → ``metron``) and resolved via platform discovery — same as + ``import --format=cyberark_portal``. """ - id_tenant_raw = environ.get("KEEPER_CYBERARK_ID_TENANT") or prompt("CyberArk Identity Tenant ID: ") + id_tenant_raw = ( + environ.get("KEEPER_CYBERARK_ID_TENANT") + or environ.get("_CYBERARK_ID_TENANT") + ) + if not id_tenant_raw: + tenant_subdomain = self._privilege_cloud_tenant_name() + discovered = self._discover_identity_endpoint(tenant_subdomain) + if discovered: + logging.info("Platform discovery resolved tenant to %s", discovered) + return discovered + if sys.stdin and sys.stdin.isatty(): + id_tenant_raw = prompt("CyberArk Identity Tenant ID: ") + else: + print_formatted_text(HTML( + "Unable to resolve the CyberArk Identity tenant. " + "Set KEEPER_CYBERARK_ID_TENANT and retry." + )) + return None + + if not id_tenant_raw: + print_formatted_text(HTML( + "CyberArk Identity Tenant ID is required. " + "Set KEEPER_CYBERARK_ID_TENANT and retry." + )) + return None + id_tenant_raw = id_tenant_raw.strip() if id_tenant_raw.startswith("https://"): id_tenant_raw = id_tenant_raw[len("https://"):] @@ -270,11 +316,8 @@ def _resolve_identity_host(self) -> Optional[str]: id_tenant_raw = id_tenant_raw.rstrip("/") if "." in id_tenant_raw: - id_host = id_tenant_raw - # But for the OAuth2 URL we need *.cyberark.cloud domain if id_tenant_raw.endswith(".my.idaptive.app"): - # Legacy Idaptive — extract subdomain for cyberark.cloud OAuth2 id_host = id_tenant_raw.split(".")[0] + ".id.cyberark.cloud" logging.info("Legacy Idaptive tenant detected, using %s for OAuth2", id_host) elif not id_tenant_raw.endswith(".cyberark.cloud"): @@ -285,13 +328,11 @@ def _resolve_identity_host(self) -> Optional[str]: id_tenant += ".id" id_host = f"{id_tenant}.cyberark.cloud" - # Validate the base portion (before first dot) base_part = id_host.split(".")[0] if not re.match(r'^[a-zA-Z0-9]+$', base_part): print_formatted_text(HTML("Invalid tenant ID format")) return None - # Platform discovery — resolve tenant to correct identity endpoint discovered_host = self._discover_identity_endpoint(base_part) if discovered_host: id_host = discovered_host @@ -300,29 +341,44 @@ def _resolve_identity_host(self) -> Optional[str]: @staticmethod def _choose_cloud_auth_method() -> str: - """Return 'service' or 'interactive' for Privilege Cloud authentication. - """ - method = (environ.get("KEEPER_CYBERARK_AUTH_METHOD") or "").strip().lower() - if method in ("interactive", "identity", "user", "mfa", "2fa", "up"): + """Return 'service' or 'interactive' for Privilege Cloud authentication.""" + method = ( + environ.get("KEEPER_CYBERARK_AUTH_METHOD") + or environ.get("_CYBERARK_AUTH_METHOD") + or "" + ).strip().lower() + if method in ("interactive", "identity", "user", "mfa", "2fa", "up", "portal", "sso", "idp"): return "interactive" if method in ("service", "service_account", "oauth", "oauth2", "client_credentials"): return "service" - print_formatted_text(HTML( - "\nCyberArk Privilege Cloud authentication method:\n" - " [1] Service account (OAuth2 client credentials)\n" - " [2] User login with MFA / 2FA (CyberArk Identity)" - )) - try: - choice = prompt("Select authentication method [1/2] (default 1): ").strip() - except (EOFError, KeyboardInterrupt): + username = environ.get("KEEPER_CYBERARK_USERNAME") or environ.get("_CYBERARK_USERNAME") + password = environ.get("KEEPER_CYBERARK_PASSWORD") or environ.get("_CYBERARK_PASSWORD") + if username and password: return "service" - return "interactive" if choice == "2" else "service" + if sys.stdin and sys.stdin.isatty(): + print_formatted_text(HTML( + "\nCyberArk Privilege Cloud authentication method:\n" + " [1] Service account (OAuth2 client credentials)\n" + " [2] User login with MFA / 2FA / SSO (CyberArk Identity)" + )) + try: + choice = prompt("Select authentication method [1/2] (default 1): ").strip() + except (EOFError, KeyboardInterrupt): + return "service" + return "interactive" if choice == "2" else "service" + return "interactive" if sys.stdin and sys.stdin.isatty() else "service" def _auth_privilege_cloud_service(self, id_host: str) -> bool: """Authenticate to Privilege Cloud via OAuth2 service account (no MFA).""" - client_id = environ.get("KEEPER_CYBERARK_USERNAME") or prompt("CyberArk service user name: ") - client_secret = environ.get("KEEPER_CYBERARK_PASSWORD") or prompt( - "CyberArk service user password: ", is_password=True + client_id = ( + environ.get("KEEPER_CYBERARK_USERNAME") + or environ.get("_CYBERARK_USERNAME") + or prompt("CyberArk service user name: ") + ) + client_secret = ( + environ.get("KEEPER_CYBERARK_PASSWORD") + or environ.get("_CYBERARK_PASSWORD") + or prompt("CyberArk service user password: ", is_password=True) ) oauth2_url = f"https://{id_host}/oauth2/platformtoken" logging.info("Authenticating to Privilege Cloud via %s", oauth2_url) @@ -342,7 +398,7 @@ def _auth_privilege_cloud_service(self, id_host: str) -> bool: )) print_formatted_text(HTML( "Tip: the OAuth2 client-credentials flow only works for " - "CyberArk service accounts. To sign in as a regular user with MFA / 2FA, " + "CyberArk service accounts. To sign in as a regular user with MFA / 2FA / SSO, " "re-run and choose authentication method 2 " "(or set KEEPER_CYBERARK_AUTH_METHOD=interactive)." )) @@ -353,7 +409,7 @@ def _auth_privilege_cloud_service(self, id_host: str) -> bool: print_formatted_text(HTML("Failed to parse OAuth2 response")) print_formatted_text(HTML( "Tip: the OAuth2 client-credentials flow only works for " - "CyberArk service accounts. To sign in as a regular user with MFA / 2FA, " + "CyberArk service accounts. To sign in as a regular user with MFA / 2FA / SSO, " "re-run and choose authentication method 2 " "(or set KEEPER_CYBERARK_AUTH_METHOD=interactive)." )) @@ -362,27 +418,42 @@ def _auth_privilege_cloud_service(self, id_host: str) -> bool: return True def _auth_privilege_cloud_interactive(self, id_host: str) -> bool: - """Authenticate to Privilege Cloud as an interactive user with MFA / 2FA. + """Authenticate to Privilege Cloud as an interactive user (MFA / 2FA / SSO). + + Uses the CyberArk Identity native-client flow (``X-IDAP-NATIVE-CLIENT`` + + ``OobIdPAuth``) so SSO users are redirected to their external IdP in + a browser, matching ark-sdk-python / IdentityCommand behavior. """ identity_base_url = f"https://{id_host}" - tenant_name = id_host.split(".")[0] - username = environ.get("KEEPER_CYBERARK_USERNAME") or prompt("CyberArk username: ") + tenant_name = self._privilege_cloud_tenant_name() + username = ( + environ.get("KEEPER_CYBERARK_USERNAME") + or environ.get("_CYBERARK_USERNAME") + or prompt("CyberArk User Portal username: ") + ) - headers = {"X-IDAP-NATIVE-CLIENT": "true"} - start_payload = {"TenantId": tenant_name, "User": username, "Version": "1.0"} + # Persist cookies across StartAuthentication → OobAuthStatus / AdvanceAuthentication. + session = requests.Session() + headers = { + "Content-Type": "application/json", + "X-IDAP-NATIVE-CLIENT": "true", + "OobIdPAuth": "true", + } + start_payload = { + "TenantId": tenant_name, + "User": username, + "Version": "1.0", + "PlatformTokenResponse": True, + } + logging.info("Using CyberArk Identity URL: %s", identity_base_url) identity_base_url, result = self._start_identity_authentication( - identity_base_url, start_payload, headers, + identity_base_url, start_payload, headers, session=session, ) if result is None: return False if result.get("IdpRedirectUrl") or result.get("IdpRedirectShortUrl"): - print_formatted_text(HTML( - "This account signs in through SSO / an external identity provider, " - "which the interactive importer does not support. Use a CyberArk service account " - "(authentication method 1) instead." - )) - return False + return self._complete_identity_sso(identity_base_url, result, session=session) session_id = result.get("SessionId") challenges = result.get("Challenges") or [] @@ -391,7 +462,7 @@ def _auth_privilege_cloud_interactive(self, id_host: str) -> bool: logging.debug("StartAuthentication result missing SessionId/Challenges") return False - password = environ.get("KEEPER_CYBERARK_PASSWORD") + password = environ.get("KEEPER_CYBERARK_PASSWORD") or environ.get("_CYBERARK_PASSWORD") advance_result = None for challenge in challenges: mechanisms = challenge.get("Mechanisms") or [] @@ -401,7 +472,8 @@ def _auth_privilege_cloud_interactive(self, id_host: str) -> bool: if mechanism is None: return False advance_result = self._answer_identity_mechanism( - identity_base_url, tenant_name, session_id, mechanism, password=password, + identity_base_url, tenant_name, session_id, mechanism, + password=password, session=session, ) if advance_result is None: return False @@ -415,7 +487,7 @@ def _auth_privilege_cloud_interactive(self, id_host: str) -> bool: )) return False - token = advance_result.get("Token") + token = advance_result.get("Token") or advance_result.get("Auth") if not token: print_formatted_text(HTML("CyberArk Identity did not return a session token")) return False @@ -423,14 +495,144 @@ def _auth_privilege_cloud_interactive(self, id_host: str) -> bool: print_formatted_text(HTML("Log on successful")) return True + def _complete_identity_sso(self, identity_base_url: str, start_result: dict, + session=None) -> bool: + """Complete SSO / external IdP authentication after StartAuthentication. + + Opens the IdP redirect URL in the browser, then either: + - polls ``/Security/OobAuthStatus`` until the IdP session succeeds, or + - prompts for an OOBAUTHPIN when ``IdpOobAuthPinRequired`` is set. + """ + redirect_url = ( + start_result.get("IdpRedirectShortUrl") + or start_result.get("IdpRedirectUrl") + or "" + ) + idp_session_id = start_result.get("IdpLoginSessionId") or "" + if not redirect_url or not idp_session_id: + print_formatted_text(HTML( + "SSO authentication response is missing the IdP redirect URL or session id" + )) + return False + + print_formatted_text(HTML( + "\nYou are being redirected to your external identity provider (SSO).\n" + "If the browser does not open, open this URL manually:\n\n" + f" {_esc(redirect_url)}\n" + )) + self._open_browser(redirect_url) + + if start_result.get("IdpOobAuthPinRequired"): + return self._complete_identity_sso_pin(identity_base_url, idp_session_id, session=session) + return self._poll_identity_sso_status(identity_base_url, idp_session_id, session=session) + + def _complete_identity_sso_pin(self, identity_base_url: str, idp_session_id: str, + session=None) -> bool: + """Complete SSO when CyberArk Identity requires an OOBAUTHPIN after IdP login.""" + print_formatted_text(HTML( + "After completing SSO in the browser, enter the PIN code shown " + "by CyberArk Identity (email / SMS / on-screen)." + )) + try: + pin_code = prompt("SSO PIN code: ").strip() + except (EOFError, KeyboardInterrupt): + return False + if not pin_code: + print_formatted_text(HTML("PIN code is required to complete SSO login")) + return False + + http = session or requests + url = f"{identity_base_url}/Security/AdvanceAuthentication" + headers = {"Content-Type": "application/json", "X-IDAP-NATIVE-CLIENT": "true"} + body = { + "SessionId": idp_session_id, + "MechanismId": "OOBAUTHPIN", + "Action": "Answer", + "Answer": pin_code, + } + try: + response = http.post(url, json=body, headers=headers, timeout=self.TIMEOUT) + except _requests_module.RequestException: + print_formatted_text(HTML("SSO PIN submission failed: connection error")) + logging.debug("OOBAUTHPIN AdvanceAuthentication connection error", exc_info=True) + return False + + result = self._identity_result(response) + if result is None: + return False + if result.get("Summary") != IDENTITY_LOGIN_SUCCESS: + summary = result.get("Summary") or result.get("State") or "unknown" + print_formatted_text(HTML( + f"SSO PIN authentication failed (status: {_esc(summary)})" + )) + return False + token = result.get("Token") or result.get("Auth") + if not token: + print_formatted_text(HTML("CyberArk Identity did not return a session token after SSO PIN")) + return False + self.auth_token = f"Bearer {token}" + print_formatted_text(HTML("Log on successful")) + return True + + def _poll_identity_sso_status(self, identity_base_url: str, idp_session_id: str, + session=None) -> bool: + """Poll ``/Security/OobAuthStatus`` until browser SSO completes.""" + http = session or requests + url = f"{identity_base_url}/Security/OobAuthStatus" + headers = {"Content-Type": "application/json", "X-IDAP-NATIVE-CLIENT": "true"} + body = {"SessionId": idp_session_id} + print_formatted_text(HTML( + "Waiting for SSO to complete in the browser…" + )) + + waited = 0 + while waited < self.IDENTITY_IDP_POLL_TIMEOUT: + try: + response = http.post(url, json=body, headers=headers, timeout=self.TIMEOUT) + except _requests_module.RequestException: + print_formatted_text(HTML("SSO status check failed: connection error")) + logging.debug("OobAuthStatus connection error", exc_info=True) + return False + + result = self._identity_result(response) + if result is None: + return False + + state = (result.get("State") or "").strip() + token = result.get("Token") or result.get("Auth") + if state == IDENTITY_IDP_STATE_SUCCESS and token: + self.auth_token = f"Bearer {token}" + print_formatted_text(HTML("Log on successful")) + return True + if state.lower() in ("failed", "error", "canceled", "cancelled", "expired"): + print_formatted_text(HTML( + f"SSO authentication failed (status: {_esc(state or 'unknown')})" + )) + return False + + time.sleep(self.IDENTITY_IDP_POLL_INTERVAL) + waited += self.IDENTITY_IDP_POLL_INTERVAL + + print_formatted_text(HTML("Timed out waiting for SSO authentication to complete")) + return False + + @staticmethod + def _open_browser(url: str) -> None: + """Best-effort open of the SSO IdP URL in the user's default browser.""" + try: + webbrowser.open(url, new=0, autoraise=True) + except Exception: + logging.debug("Failed to open browser for SSO URL", exc_info=True) + def _start_identity_authentication(self, identity_base_url: str, start_payload: dict, - headers: dict) -> Tuple[Optional[str], Optional[dict]]: + headers: dict, session=None) -> Tuple[Optional[str], Optional[dict]]: """POST ``/Security/StartAuthentication``, following HTTP and pod redirects. """ + http = session or requests url = f"{identity_base_url}/Security/StartAuthentication" for _ in range(4): try: - response = requests.post( + response = http.post( url, json=start_payload, headers=headers, timeout=self.TIMEOUT, allow_redirects=False, ) @@ -517,9 +719,11 @@ def _select_identity_mechanism(mechanisms: List[dict]) -> Optional[dict]: def _answer_identity_mechanism(self, identity_base_url: str, tenant_name: str, session_id: str, mechanism: dict, - password: Optional[str] = None) -> Optional[dict]: + password: Optional[str] = None, + session=None) -> Optional[dict]: """Drive one CyberArk Identity challenge mechanism to completion. """ + http = session or requests url = f"{identity_base_url}/Security/AdvanceAuthentication" headers = {"X-IDAP-NATIVE-CLIENT": "true"} name = mechanism.get("Name") or "" @@ -531,7 +735,7 @@ def _answer_identity_mechanism(self, identity_base_url: str, tenant_name: str, def _post(body: dict) -> Optional[dict]: try: - resp = requests.post(url, json=body, headers=headers, timeout=self.TIMEOUT) + resp = http.post(url, json=body, headers=headers, timeout=self.TIMEOUT) except _requests_module.RequestException as e: print_formatted_text(HTML("Authentication request failed: connection error")) logging.debug("AdvanceAuthentication connection error: %s", type(e).__name__) @@ -541,7 +745,7 @@ def _post(body: dict) -> Optional[dict]: # Text answer — password (UP) or a typed code (OTP / authenticator). if name == "UP" or answer_type == "text": if name == "UP": - answer = password or prompt("CyberArk password: ", is_password=True) + answer = password or prompt("CyberArk Identity Portal password: ", is_password=True) else: answer = prompt(f"{prompt_label}: ") return _post(dict(base, Action="Answer", Answer=answer)) @@ -1206,7 +1410,7 @@ def retrieve_password(self, account_id: str, account_name: str = "", self._get_url("account_password").format(account_id=account_id), headers={"Authorization": self.auth_token, "Content-Type": "application/json"}, json={ - "reason": "Keeper Commander Import", + "reason": "test", **({"TicketingSystemName": environ["KEEPER_CYBERARK_TICKETING_SYSTEM"]} if "KEEPER_CYBERARK_TICKETING_SYSTEM" in environ else {}), **({"TicketId": environ["KEEPER_CYBERARK_TICKET_ID"]} @@ -1253,4 +1457,3 @@ def retrieve_password(self, account_id: str, account_name: str = "", print_formatted_text(HTML(f"Password retrieval aborted (status {response.status_code})")) return None return None - diff --git a/keepercommander/importer/cyberark/pam/constants.py b/keepercommander/importer/cyberark/pam/constants.py index 216c76352..b366f7a66 100644 --- a/keepercommander/importer/cyberark/pam/constants.py +++ b/keepercommander/importer/cyberark/pam/constants.py @@ -82,6 +82,9 @@ def reset_system_safes() -> None: # CyberArk Identity AdvanceAuthentication Summary for a completed login IDENTITY_LOGIN_SUCCESS = "LoginSuccess" +# CyberArk Identity OOB IdP (SSO) poll status for a completed login +IDENTITY_IDP_STATE_SUCCESS = "Success" + # Default CyberArk platformId → KeeperPAM record mapping DEFAULT_PLATFORM_MAP = { # NIX diff --git a/keepercommander/importer/cyberark_portal/cyberark_portal.py b/keepercommander/importer/cyberark_portal/cyberark_portal.py index fb9601d92..119b0c9cc 100644 --- a/keepercommander/importer/cyberark_portal/cyberark_portal.py +++ b/keepercommander/importer/cyberark_portal/cyberark_portal.py @@ -15,7 +15,7 @@ from tabulate import tabulate -from ..importer import BaseImporter, Folder, Record, RecordField +from ..importer import BaseImporter, Folder, PathDelimiter, Record, RecordField, SharedFolder import secrets import string @@ -118,6 +118,7 @@ class CyberArkPortalImporter(BaseImporter): Keeper login type records for Applications and Passwords and secure note records for SecuredItems. """ + verbose_import_summary = True LOOP_DELAY = 0.025 # Use quarter millisecond delay between requests to avoid hitting the API rate limits TIMEOUT = 10 # Wait up to 10 seconds for CyberArk API requests @@ -282,6 +283,18 @@ def _folder_display_name(folder): or folder.get("Title") or folder.get("CollectionName") or folder.get("ID") or "") + @staticmethod + def _folders_from_up_data(up_result): + """Pull Identity collections/folders from GetUPData when present.""" + folders = [] + if not isinstance(up_result, dict): + return folders + for key in ("Collections", "Folders", "UserCollections"): + items = up_result.get(key) + if isinstance(items, list): + folders.extend(x for x in items if isinstance(x, dict)) + return folders + def _apply_sharing_and_folders(self, *, record, item, item_key, folder_index, identity_base_url, authentication_token, missing_endpoint_cache, item_kind): @@ -322,7 +335,7 @@ def _apply_sharing_and_folders(self, *, record, item, item_key, folder_index, if not fname: continue rec_folder = Folder() - rec_folder.path = fname + rec_folder.path = fname.replace(PathDelimiter, 2 * PathDelimiter) # Map CyberArk's coarse folder permissions onto Keeper's perm = (f.get("Permission") or f.get("AccessLevel") or f.get("EffectivePermission") or "") @@ -396,6 +409,7 @@ def discover_identity_url(tenant_name): return default_url def do_import(self, filename, **kwargs): + use_nsf = bool(kwargs.get("use_nsf")) name = filename.removeprefix("https://").removeprefix("http://") host_part = name.split("/")[0] @@ -408,6 +422,13 @@ def do_import(self, filename, **kwargs): identity_base_url = self.discover_identity_url(tenant_name) logging.info(f"Using CyberArk Identity URL: {identity_base_url}") + if use_nsf: + print_formatted_text( + HTML( + "\nNSF mode: CyberArk Identity folders will be created as " + "Nested Share Folders and items as NSF records." + ) + ) username = environ.get("KEEPER_CYBERARK_USERNAME") or prompt("CyberArk User Portal username: ") @@ -711,17 +732,39 @@ def complete(self): if response.status_code != HTTPStatus.OK: logging.error(f"HTTP {HTTPStatus(response.status_code).phrase} error getting UP data: {response.text}") return - apps = response.json()["Result"]["Apps"] + up_result = response.json().get("Result") or {} + apps = up_result.get("Apps") or [] - missing_endpoint_cache = set() - folders = self._fetch_folders(identity_base_url, authentication_token, missing_endpoint_cache) + folders = list(self._folders_from_up_data(up_result)) + fetched_folders = self._fetch_folders(identity_base_url, authentication_token, missing_endpoint_cache) + if fetched_folders: + folders.extend(fetched_folders) + deduped_folders = {} + for folder in folders: + key = self._folder_display_name(folder).strip().lower() + if key: + deduped_folders.setdefault(key, folder) + folders = list(deduped_folders.values()) folder_index = self._build_folder_index(folders) if folders else {} if folders: print_formatted_text( HTML(f"Discovered {len(folders)} CyberArk folder(s) for the current user."), end="\n\n", ) + if use_nsf: + seen_names = set() + for folder in folders: + fname = self._folder_display_name(folder) + if not fname: + continue + key = fname.lower() + if key in seen_names: + continue + seen_names.add(key) + nsf_folder = SharedFolder() + nsf_folder.path = fname.replace(PathDelimiter, 2 * PathDelimiter) + yield nsf_folder if len(apps) > 0: print_formatted_text( @@ -852,5 +895,3 @@ def complete(self): record.fields.append(RecordField(type="note", value=itemData["n"])) yield record - - print_formatted_text(HTML("Import complete")) diff --git a/keepercommander/importer/imp_exp.py b/keepercommander/importer/imp_exp.py index 1e08d8069..fcaa869dd 100644 --- a/keepercommander/importer/imp_exp.py +++ b/keepercommander/importer/imp_exp.py @@ -753,15 +753,20 @@ def _import(params, file_format, filename, **kwargs): secret_ids = kwargs.get('secret_ids') target_node = kwargs.get('target_node') - import_into = kwargs.get('import_into') or '' + import_into_raw = kwargs.get('import_into') or '' + import_into = import_into_raw if import_into: import_into = import_into.replace(PathDelimiter, 2*PathDelimiter) update_flag = kwargs.get('update_flag') or False no_shortcuts = kwargs.get('no_shortcuts') or False importer = importer_for_format(file_format)() # type: BaseImporter + verbose_import_summary = importer.verbose_import_summary + show_skipped = show_skipped or verbose_import_summary records_before = len(params.record_cache) + skipped_existing_count = 0 + successful_import_count = 0 folders = [] # type: List[ImportSharedFolder] records = [] # type: List[ImportRecord] @@ -774,7 +779,7 @@ def _import(params, file_format, filename, **kwargs): for x in importer.execute(filename, params=params, users_only=import_users, filter_folder=filter_folder, old_domain=old_domain, new_domain=new_domain, tmpdir=tmpdir, secret_ids=secret_ids, - dry_run=dry_run, target_node=target_node): + dry_run=dry_run, target_node=target_node, use_nsf=use_nsf): if isinstance(x, ImportRecord): if filter_folder and not importer.support_folder_filter(): if not x.folders: @@ -841,6 +846,8 @@ def _import(params, file_format, filename, **kwargs): folders.append(x) + declared_nsf_folders = list(folders) + manage_users = kwargs.get('manage_users') or False manage_records = kwargs.get('manage_records') or False can_edit = kwargs.get('can_edit') or False @@ -861,7 +868,7 @@ def _import(params, file_format, filename, **kwargs): nsf_base_parent = '' if import_into: - from .nsf_import import resolve_nsf_folder + from .nsf_import import is_nsf_folder, resolve_nsf_folder resolved_nsf = resolve_nsf_folder(params, import_into) if resolved_nsf: use_nsf = True @@ -869,10 +876,61 @@ def _import(params, file_format, filename, **kwargs): if import_into == resolved_nsf: nsf_base_parent = resolved_nsf _strip_path_prefix(records, folders, import_into) + elif use_nsf: + target_folder, unresolved = try_resolve_path(params, import_into_raw) + if target_folder and not unresolved and not is_nsf_folder(params, target_folder.uid): + raise CommandError( + 'import', + '--nsf cannot import into a classic folder. Choose a Nested Share Folder target.', + ) if use_nsf: - from .nsf_import import flatten_record_folder_paths + from .nsf_import import ensure_nsf_record_folders, flatten_record_folder_paths flatten_record_folder_paths(records) + default_nsf = import_into if (import_into and not nsf_base_parent) else None + ensure_nsf_record_folders(records, folders, default_nsf) + + # Importers with verbose summaries also emit a clear message when the target + # folder(s) already exist in Keeper. + if verbose_import_summary: + if use_nsf: + from .nsf_import import resolve_nsf_folder + + requested_paths = {} # type: Dict[Tuple[str, ...], str] + for sf in folders or []: + if getattr(sf, 'path', None): + comps = tuple(path_components(sf.path)) + requested_paths[tuple(x.casefold() for x in comps)] = PathDelimiter.join( + x.replace(PathDelimiter, 2 * PathDelimiter) for x in comps) + for rec in records or []: + for fol in rec.folders or []: + if getattr(fol, 'path', None): + comps = tuple(path_components(fol.path)) + requested_paths[tuple(x.casefold() for x in comps)] = PathDelimiter.join( + x.replace(PathDelimiter, 2 * PathDelimiter) for x in comps) + elif getattr(fol, 'domain', None): + comps = tuple(path_components(fol.domain)) + requested_paths[tuple(x.casefold() for x in comps)] = PathDelimiter.join( + x.replace(PathDelimiter, 2 * PathDelimiter) for x in comps) + + existing_paths = [] + for path_key, display_path in sorted(requested_paths.items()): + if not path_key: + continue + try: + uid = resolve_nsf_folder(params, display_path) + except (CommandError, KeeperApiError, ValueError, KeyError) as exc: + logging.debug('Unable to resolve NSF folder "%s": %s', display_path, exc) + uid = None + if uid: + existing_paths.append(display_path) + + if existing_paths: + for p in existing_paths: + logging.info( + 'Nested Share Folder "%s" already exists in Keeper - skipping creation', + p, + ) if classic_shared: sfol = set() @@ -931,6 +989,57 @@ def _import(params, file_format, filename, **kwargs): fol.domain = sf_map[sf_name] fol.path = (path[len(sf_name):]).strip(PathDelimiter) + if verbose_import_summary and not use_nsf: + # For classic shared/user folders, use a conservative check based on + # folder paths (from cached Keeper folder tree). + requested_folders = {} # type: Dict[Tuple[str, ...], str] + for sf in folders or []: + if getattr(sf, 'path', None): + comps = tuple(path_components(sf.path)) + requested_folders[tuple(x.casefold() for x in comps)] = PathDelimiter.join( + x.replace(PathDelimiter, 2 * PathDelimiter) for x in comps) + for rec in records or []: + for fol in rec.folders or []: + comps = [] + if getattr(fol, 'domain', None): + comps.extend(path_components(fol.domain)) + if getattr(fol, 'path', None): + comps.extend(path_components(fol.path)) + if comps: + key = tuple(x.casefold() for x in comps) + requested_folders[key] = PathDelimiter.join( + x.replace(PathDelimiter, 2 * PathDelimiter) for x in comps) + + existing_folder_paths = set() # type: Set[str] + for uid, node in (params.folder_cache or {}).items(): + # Root doesn't have a meaningful name in import requests. + if not node or getattr(node, 'type', None) == BaseFolderNode.RootFolderType: + continue + try: + p = get_folder_path(params, uid).strip(PathDelimiter) + except Exception: + continue + if p: + existing_folder_paths.add(p) + + existing_folder_keys = { + tuple(x.casefold() for x in path_components(p)) + for p in existing_folder_paths + } + + already_exists = [] + for path_key, display_path in requested_folders.items(): + if path_key in existing_folder_keys: + already_exists.append(display_path) + + already_exists = sorted(set(already_exists)) + if already_exists: + for p in already_exists: + logging.info( + 'Folder "%s" already exists in Keeper - records will be added to the existing folder', + p, + ) + if use_nsf: from .nsf_import import prepare_nsf_folders if not dry_run: @@ -954,6 +1063,7 @@ def _import(params, file_format, filename, **kwargs): import_uids = {} records_to_import, record_exists, external_lookup = prepare_record_add_or_update(update_flag, no_shortcuts, params, records) + skipped_existing_count = len(record_exists) if show_skipped and record_exists: for existing_record in record_exists: folder_name = '' @@ -964,7 +1074,18 @@ def _import(params, file_format, filename, **kwargs): if f.path: folder_name += f.path - if folder_name: + if verbose_import_summary: + if folder_name: + logging.info( + 'Record "%s" in folder "%s" already exists in Keeper [%s] - skipped.', + existing_record.title, folder_name, existing_record.uid, + ) + else: + logging.info( + 'Record "%s" already exists in Keeper [%s] - skipped.', + existing_record.title, existing_record.uid, + ) + elif folder_name: logging.info('Record "%s" appearing in Folder "%s" was skipped due to a duplicate record [%s] found.', existing_record.title, folder_name, existing_record.uid) else: @@ -1096,7 +1217,13 @@ def _import(params, file_format, filename, **kwargs): folder_uid = '' from .nsf_import import is_nsf_folder - if folder_uid and is_nsf_folder(params, folder_uid): + use_nsf_add = use_nsf or (folder_uid and is_nsf_folder(params, folder_uid)) + if use_nsf_add: + if folder_uid and not is_nsf_folder(params, folder_uid): + logging.warning( + 'Skipping NSF record "%s": Nested Share Folder was not created', + import_record.title) + continue data = _construct_record_v3_data(import_record) data_bytes = api.get_record_data_json_bytes(data) if len(data_bytes) > RECORD_MAX_DATA_LEN: @@ -1174,9 +1301,13 @@ def _import(params, file_format, filename, **kwargs): if nsf_records_to_add: from .nsf_import import execute_nsf_records_add - execute_nsf_records_add(params, nsf_records_to_add) + nsf_rs = execute_nsf_records_add(params, nsf_records_to_add) + successful_import_count += sum( + 1 for result in nsf_rs if result.status == record_pb2.RS_SUCCESS) if records_v3_to_add: - rec_rs = execute_records_add(params, records_v3_to_add) + add_rs = execute_records_add(params, records_v3_to_add) + successful_import_count += sum( + 1 for result in add_rs if result.status == record_pb2.RS_SUCCESS) if records_v2_to_update: execute_update_v2_record(params, records_v2_to_update) if records_v3_to_update: @@ -1298,18 +1429,36 @@ def _import(params, file_format, filename, **kwargs): if len(v3_atts) > 0: upload_v3_attachments(params, v3_atts) - if use_nsf and folders and not dry_run: + if use_nsf and declared_nsf_folders and not dry_run: from .nsf_import import apply_nsf_folder_permissions apply_nsf_folder_permissions( - params, folders, manage_users, manage_records, can_edit, can_share) + params, declared_nsf_folders, manage_users, manage_records, can_edit, can_share) if hasattr(importer, 'cleanup') and callable(importer.cleanup): importer.cleanup() records_after = len(params.record_cache) - if records_after > records_before: + imported_count = max(0, records_after - records_before) + if imported_count > 0: params.queue_audit_event('imported_records', file_format=file_format.upper()) - logging.info("%d records imported successfully", records_after - records_before) + if not verbose_import_summary: + logging.info("%d records imported successfully", imported_count) + + if verbose_import_summary and not dry_run: + if successful_import_count == 0 and skipped_existing_count > 0: + logging.info( + 'Import finished: no new records imported; %d record(s) already exist in Keeper.', + skipped_existing_count, + ) + elif successful_import_count > 0 and skipped_existing_count > 0: + logging.info( + 'Import finished: %d record(s) imported; %d record(s) already exist in Keeper (skipped).', + successful_import_count, skipped_existing_count, + ) + elif successful_import_count > 0: + logging.info('Import finished: %d record(s) imported successfully.', successful_import_count) + else: + logging.info('Import finished: no records were imported.') def report_statuses(status_type, status_iter): @@ -2307,8 +2456,11 @@ def prepare_record_link(params, records): """Prepare record links to folders.""" record_folders = {} # type: [str, [str]] record_links = [] + nsf_records = getattr(params, 'nested_share_records', None) or {} for rec in records: if rec.uid: + if rec.uid in nsf_records: + continue if rec.uid in params.record_cache: if rec.uid in record_folders: folder_ids = record_folders[rec.uid] @@ -2327,8 +2479,11 @@ def prepare_record_link(params, records): if folder_uid in folder_ids: continue if len(folder_ids) > 0: + src_uid = folder_ids[0] + if src_uid not in params.folder_cache or is_nsf_folder(params, src_uid): + continue folder_ids.append(folder_uid) - src_folder = params.folder_cache[folder_ids[0]] + src_folder = params.folder_cache[src_uid] dst_folder = params.folder_cache[folder_uid] if folder_uid in params.folder_cache else params.root_folder ft = dst_folder.type if dst_folder.type != BaseFolderNode.RootFolderType else BaseFolderNode.UserFolderType req = { diff --git a/keepercommander/importer/importer.py b/keepercommander/importer/importer.py index 9742b5119..32aad3139 100644 --- a/keepercommander/importer/importer.py +++ b/keepercommander/importer/importer.py @@ -292,6 +292,8 @@ def __init__(self): class BaseImporter(abc.ABC): + verbose_import_summary = False + def execute(self, name, **kwargs): # type: (BaseImporter, str, ...) -> Iterable[Union[Record, SharedFolder, File]] yield from self.do_import(name, **kwargs) diff --git a/keepercommander/importer/nsf_import.py b/keepercommander/importer/nsf_import.py index a3d79fd90..5181e2bb5 100644 --- a/keepercommander/importer/nsf_import.py +++ b/keepercommander/importer/nsf_import.py @@ -17,6 +17,7 @@ from typing import Dict, Iterable, List, Optional, Tuple from .importer import ( + Folder as ImportFolder, PathDelimiter, Permission as ImportPermission, Record as ImportRecord, @@ -93,6 +94,34 @@ def flatten_record_folder_paths(records): # type: (List[ImportRecord]) -> None ) if comps else '' +def ensure_nsf_record_folders(records, folders, default_name=None): + # type: (List[ImportRecord], List[ImportSharedFolder], Optional[str]) -> None + """Ensure record folder paths also have NSF folder targets.""" + name = (default_name or '').strip() + existing = {(fol.path or '').lower() for fol in folders or [] if fol.path} + + def _add_shared(path): + key = (path or '').lower() + if not path or key in existing: + return + sf = ImportSharedFolder() + sf.path = path + folders.append(sf) + existing.add(key) + + for rec in records or []: + if not rec.folders: + if not name: + continue + rec.folders = [ImportFolder()] + for fol in rec.folders: + if not (fol.path or fol.domain): + if not name: + continue + fol.path = name + _add_shared(fol.path or fol.domain) + + def find_nsf_child(params, folder_name, parent_uid): # type: (KeeperParams, str, Optional[str]) -> Optional[str] """Find an NSF child named *folder_name* under *parent_uid* (or vault root).""" @@ -309,22 +338,23 @@ def prepare_nsf_folders(params, folders, records, base_parent_uid=''): def build_nsf_record_add(params, import_record, record_key, data): - """Build a ``vault/records/v3/add`` RecordAdd for an NSF folder.""" + """Build a ``vault/records/v3/add`` RecordAdd.""" from ..nested_share_folder.common import get_folder_key from ..nested_share_folder.record_api import create_record_data_v3 folder_uid = '' if import_record.folders: folder_uid = import_record.folders[0].uid or '' - if not folder_uid or not is_nsf_folder(params, folder_uid): - raise CommandError('import', f'NSF folder not found for record "{import_record.title}"') - - folder_key = get_folder_key(params, folder_uid, raise_on_missing=True) + folder_key = None + if folder_uid: + if not is_nsf_folder(params, folder_uid): + raise CommandError('import', f'NSF folder not found for record "{import_record.title}"') + folder_key = get_folder_key(params, folder_uid, raise_on_missing=True) return create_record_data_v3( record_uid=import_record.uid, record_key=record_key, data=data, - folder_uid=folder_uid, + folder_uid=folder_uid or None, folder_key=folder_key, data_key=params.data_key, client_modified_time=utils.current_milli_time(), diff --git a/tests/test_cyberark_pam_import.py b/tests/test_cyberark_pam_import.py index 4f969bd9e..433944469 100644 --- a/tests/test_cyberark_pam_import.py +++ b/tests/test_cyberark_pam_import.py @@ -653,6 +653,162 @@ def test_invalid_login_type_rejected_by_authenticate(self, mock_dns, mock_prompt assert result is False +# ── Privilege Cloud Interactive / SSO Auth Tests ───────────── + + +class TestPrivilegeCloudSSOAuth: + + def _client(self): + return CyberArkPVWAClient("mycompany.privilegecloud.cyberark.cloud") + + def test_choose_auth_method_sso_alias(self, monkeypatch): + monkeypatch.setenv("KEEPER_CYBERARK_AUTH_METHOD", "sso") + assert CyberArkPVWAClient._choose_cloud_auth_method() == "interactive" + + def test_choose_auth_method_idp_alias(self, monkeypatch): + monkeypatch.setenv("KEEPER_CYBERARK_AUTH_METHOD", "idp") + assert CyberArkPVWAClient._choose_cloud_auth_method() == "interactive" + + def test_poll_identity_sso_status_success(self, monkeypatch): + client = self._client() + client.IDENTITY_IDP_POLL_INTERVAL = 0 + responses = [ + {"success": True, "Result": {"State": "Pending"}}, + {"success": True, "Result": {"State": "Success", "Token": "sso-token-123"}}, + ] + + class _Resp: + def __init__(self, body): + self.status_code = 200 + self._body = body + + def json(self): + return self._body + + class _Session: + def __init__(self): + self.calls = 0 + + def post(self, *args, **kwargs): + body = responses[min(self.calls, len(responses) - 1)] + self.calls += 1 + return _Resp(body) + + session = _Session() + monkeypatch.setattr( + "keepercommander.importer.cyberark.cyberark_pam.time.sleep", + lambda *_: None, + ) + with patch("keepercommander.importer.cyberark.cyberark_pam.print_formatted_text"): + ok = client._poll_identity_sso_status( + "https://tenant.id.cyberark.cloud", "idp-session", session=session, + ) + assert ok is True + assert client.auth_token == "Bearer sso-token-123" + assert session.calls == 2 + + def test_poll_identity_sso_status_failed_state(self): + client = self._client() + + class _Resp: + status_code = 200 + + def json(self): + return {"success": True, "Result": {"State": "Failed"}} + + class _Session: + def post(self, *args, **kwargs): + return _Resp() + + with patch("keepercommander.importer.cyberark.cyberark_pam.print_formatted_text"): + ok = client._poll_identity_sso_status( + "https://tenant.id.cyberark.cloud", "idp-session", session=_Session(), + ) + assert ok is False + assert client.auth_token is None + + def test_complete_identity_sso_pin_success(self, monkeypatch): + client = self._client() + monkeypatch.setattr( + "keepercommander.importer.cyberark.cyberark_pam.prompt", + lambda *a, **k: "123456", + ) + + class _Resp: + status_code = 200 + + def json(self): + return { + "success": True, + "Result": {"Summary": "LoginSuccess", "Token": "pin-token"}, + } + + posted = {} + + class _Session: + def post(self, url, json=None, headers=None, timeout=None): + posted["url"] = url + posted["json"] = json + return _Resp() + + with patch("keepercommander.importer.cyberark.cyberark_pam.print_formatted_text"): + ok = client._complete_identity_sso_pin( + "https://tenant.id.cyberark.cloud", "idp-login-session", session=_Session(), + ) + assert ok is True + assert client.auth_token == "Bearer pin-token" + assert posted["json"]["MechanismId"] == "OOBAUTHPIN" + assert posted["json"]["SessionId"] == "idp-login-session" + assert posted["json"]["Answer"] == "123456" + + def test_complete_identity_sso_opens_browser_and_polls(self, monkeypatch): + client = self._client() + opened = [] + + monkeypatch.setattr(CyberArkPVWAClient, "_open_browser", staticmethod(lambda url: opened.append(url))) + monkeypatch.setattr( + CyberArkPVWAClient, + "_poll_identity_sso_status", + lambda self, *a, **k: setattr(self, "auth_token", "Bearer from-poll") or True, + ) + + start_result = { + "IdpRedirectShortUrl": "https://idp.example/sso", + "IdpLoginSessionId": "idp-sess", + "IdpOobAuthPinRequired": False, + } + with patch("keepercommander.importer.cyberark.cyberark_pam.print_formatted_text"): + ok = client._complete_identity_sso( + "https://tenant.id.cyberark.cloud", start_result, session=object(), + ) + assert ok is True + assert opened == ["https://idp.example/sso"] + assert client.auth_token == "Bearer from-poll" + + def test_complete_identity_sso_uses_pin_when_required(self, monkeypatch): + client = self._client() + monkeypatch.setattr(CyberArkPVWAClient, "_open_browser", staticmethod(lambda url: None)) + called = {"pin": False} + + def _pin(self, *a, **k): + called["pin"] = True + self.auth_token = "Bearer pin" + return True + + monkeypatch.setattr(CyberArkPVWAClient, "_complete_identity_sso_pin", _pin) + start_result = { + "IdpRedirectUrl": "https://idp.example/long", + "IdpLoginSessionId": "idp-sess", + "IdpOobAuthPinRequired": True, + } + with patch("keepercommander.importer.cyberark.cyberark_pam.print_formatted_text"): + ok = client._complete_identity_sso( + "https://tenant.id.cyberark.cloud", start_result, + ) + assert ok is True + assert called["pin"] is True + + # ── SSL Verification Tests ─────────────────────────────────── diff --git a/unit-tests/test_nsf_import.py b/unit-tests/test_nsf_import.py index 520686a2b..b16f013dd 100644 --- a/unit-tests/test_nsf_import.py +++ b/unit-tests/test_nsf_import.py @@ -231,3 +231,93 @@ def test_parser_and_path_resolve(self): self.assertEqual( os.path.abspath(KeeperCsvImporter().resolve_file_path(csv_path)), os.path.abspath(csv_path)) + + def test_cyberark_style_nsf_folder_paths(self): + """CyberArk --nsf places safes as NSF paths (not classic shared domains).""" + rec = Record() + fol = Folder() + fol.path = 'RootSafe' + rec.folders = [fol] + sf = SharedFolder() + sf.path = 'RootSafe' + + nsf_import.flatten_record_folder_paths([rec]) + self.assertEqual((fol.domain or '', fol.path), ('', 'RootSafe')) + + params = _params({}) + with mock.patch(CREATE, return_value=['safe_uid']) as m: + nsf_import.prepare_nsf_folders(params, [sf], [rec]) + m.assert_called_once_with(params, [('RootSafe', '')]) + self.assertEqual(fol.uid, 'safe_uid') + self.assertEqual(sf.uid, 'safe_uid') + + def test_cyberark_portal_style_nsf_folder_paths(self): + """CyberArk Portal --nsf uses Identity folder names as NSF paths.""" + rec = Record() + fol = Folder() + fol.path = 'Identity Collection' + rec.folders = [fol] + sf = SharedFolder() + sf.path = 'Identity Collection' + + params = _params({}) + with mock.patch(CREATE, return_value=['portal_uid']) as m: + nsf_import.prepare_nsf_folders(params, [sf], [rec]) + m.assert_called_once_with(params, [('Identity Collection', '')]) + self.assertEqual((fol.uid, sf.uid), ('portal_uid', 'portal_uid')) + + def test_classic_cyberark_domain_flattens_for_nsf(self): + """Legacy domain placement still flattens correctly when --nsf is used.""" + rec = Record() + fol = Folder() + fol.domain = 'LegacySafe' + rec.folders = [fol] + nsf_import.flatten_record_folder_paths([rec]) + self.assertEqual((fol.domain, fol.path), ('', 'LegacySafe')) + + def test_ensure_nsf_record_folders_leaves_portal_root_empty(self): + rec = Record() + rec.title = 'GitHub' + folders = [] + nsf_import.ensure_nsf_record_folders([rec], folders) + self.assertFalse(rec.folders) + self.assertEqual(folders, []) + + def test_ensure_nsf_record_folders_keeps_cyberark_safe_path(self): + rec = Record() + fol = Folder() + fol.path = 'RootSafe' + rec.folders = [fol] + folders = [] + result = nsf_import.ensure_nsf_record_folders([rec], folders) + self.assertEqual(fol.path, 'RootSafe') + self.assertEqual(folders[0].path, 'RootSafe') + self.assertIsNone(result) + + def test_build_nsf_record_add_at_vault_root(self): + rec = Record() + rec.uid = 'rec1' + rec.title = 'GitHub' + params = _params() + params.data_key = b'\x00' * 32 + with mock.patch('keepercommander.nested_share_folder.record_api.create_record_data_v3', + return_value='payload') as create: + payload = nsf_import.build_nsf_record_add(params, rec, b'\x01' * 32, {'title': 'GitHub'}) + self.assertEqual(payload, 'payload') + kwargs = create.call_args.kwargs + self.assertIsNone(kwargs['folder_uid']) + self.assertIsNone(kwargs['folder_key']) + self.assertEqual(kwargs['data_key'], params.data_key) + + def test_prepare_record_link_skips_nsf_root_records(self): + from keepercommander.importer.imp_exp import prepare_record_link + rec = Record() + rec.uid = 'nsf_rec' + rec.title = 'GitHub' + params = _params() + params.nested_share_records = {'nsf_rec': {}} + params.record_cache = {'nsf_rec': {'record_key_unencrypted': b'\x00' * 32}} + params.subfolder_record_cache = {'AAAAAAAAAAAAAAAAAUIpTQ': {'nsf_rec'}} + params.folder_cache = {} + params.root_folder = mock.MagicMock(type='/', uid='') + self.assertEqual(prepare_record_link(params, [rec]), [])