diff --git a/keepercommander/commands/record_edit.py b/keepercommander/commands/record_edit.py index 6836a853a..2f04e7136 100644 --- a/keepercommander/commands/record_edit.py +++ b/keepercommander/commands/record_edit.py @@ -60,6 +60,8 @@ help='Email configuration name to use for sending (required with --send-email)') record_add_parser.add_argument('--email-message', dest='email_message', action='store', help='Custom message to include in onboarding email') +record_add_parser.add_argument('--format', dest='format', action='store', choices=['text', 'json'], + default='text', help='output format') record_add_parser.add_argument('fields', nargs='*', type=str, help='load record type data from strings with dot notation') @@ -846,6 +848,18 @@ class RecordAddCommand(Command, RecordEditMixin): def __init__(self): super(RecordAddCommand, self).__init__() + @staticmethod + def _format_add_result(record_uid: str, share_url: Optional[str] = None, + output_format: str = 'text') -> str: + data = {'record_uid': record_uid} + if share_url: + data['share_url'] = share_url + if output_format == 'json': + return json.dumps(data) + if share_url: + return f'{record_uid}\n{share_url}' + return record_uid + def get_parser(self): return record_add_parser @@ -1035,11 +1049,11 @@ def execute(self, params, **kwargs): logging.warning(f'[EMAIL] Failed to send onboarding email: {e}') # Best-effort: continue and return share URL + output_format = kwargs.get('format') or 'text' if share_url: - return share_url - else: - BreachWatch.scan_and_update_security_data(params, record.record_uid, params.breach_watch) - return record.record_uid + return self._format_add_result(record.record_uid, share_url, output_format) + BreachWatch.scan_and_update_security_data(params, record.record_uid, params.breach_watch) + return self._format_add_result(record.record_uid, None, output_format) def _sync_password_to_pam(self, params: KeeperParams, record: vault.TypedRecord, pam_config_name: str): """ diff --git a/keepercommander/service/api/onboarding.py b/keepercommander/service/api/onboarding.py index aefa88c95..5ad955ba2 100644 --- a/keepercommander/service/api/onboarding.py +++ b/keepercommander/service/api/onboarding.py @@ -17,6 +17,7 @@ from flask import Blueprint, request, jsonify, Response from typing import Tuple, Union +import json import logging from ... import api @@ -433,16 +434,18 @@ def onboard_employee(**kwargs) -> Tuple[Response, int]: 'send_email': send_email, 'email_config': email_config, 'email_message': data.get('email_message'), + 'format': 'json', 'force': True } # Execute record-add command cmd = RecordAddCommand() - share_url = cmd.execute(params, **cmd_kwargs) + result = json.loads(cmd.execute(params, **cmd_kwargs)) return jsonify({ "success": True, - "share_url": share_url, + "record_uid": result.get('record_uid'), + "share_url": result.get('share_url'), "message": f"Employee onboarded successfully" }), 201 diff --git a/keepercommander/service/util/command_util.py b/keepercommander/service/util/command_util.py index 7cac25d3d..35434b697 100644 --- a/keepercommander/service/util/command_util.py +++ b/keepercommander/service/util/command_util.py @@ -16,7 +16,7 @@ from typing import Any, Tuple, Optional from .config_reader import ConfigReader from .exceptions import CommandExecutionError -from .parse_keeper_response import parse_keeper_response +from .parse_keeper_response import parse_keeper_response, ensure_record_add_json_format from ..core.globals import get_current_params from ..decorators.logging import logger, debug_decorator, sanitize_debug_data from ... import cli, utils @@ -129,7 +129,7 @@ def execute(cls, command: str) -> Tuple[Any, int]: if params: params.service_mode = True - command = html.unescape(command) + command = ensure_record_add_json_format(html.unescape(command)) return_value, printed_output, log_output = CommandExecutor.capture_output_and_logs(params, command) response = return_value if return_value else printed_output diff --git a/keepercommander/service/util/parse_keeper_response.py b/keepercommander/service/util/parse_keeper_response.py index aae9159d7..bacd347c0 100644 --- a/keepercommander/service/util/parse_keeper_response.py +++ b/keepercommander/service/util/parse_keeper_response.py @@ -610,13 +610,20 @@ def _parse_record_add_command(response: str) -> Dict[str, Any]: "command": "record-add", "data": None } - - # Check if response is a share URL (starts with https:// and contains /vault/share) - if response_str.startswith('https://') and '/vault/share' in response_str: + + lines = [line.strip() for line in response_str.splitlines() if line.strip()] + if (len(lines) == 2 and lines[1].startswith('https://') + and '/vault/share' in lines[1]): + result["data"] = { + "record_uid": lines[0], + "share_url": lines[1] + } + # Check if response is a share URL only (legacy single-line output) + elif response_str.startswith('https://') and '/vault/share' in response_str: result["data"] = { "share_url": response_str } - elif re.match(r'^[a-zA-Z0-9_-]+$', response_str): + elif re.match(r'^[A-Za-z0-9_+/=-]+$', response_str): result["data"] = { "record_uid": response_str } @@ -1213,6 +1220,14 @@ def _filter_login_messages(response_str: str) -> str: +def ensure_record_add_json_format(command: str) -> str: + if not command.strip().startswith('record-add'): + return command + if '--format=json' in command or '--format json' in command: + return command + return f'{command} --format=json' + + def parse_keeper_response(command: str, response: Any, log_output: str = None) -> Dict[str, Any]: """ Main entry point for parsing Keeper Commander responses. diff --git a/unit-tests/service/test_response_parser.py b/unit-tests/service/test_response_parser.py index 4b389eb9a..8512c1a98 100644 --- a/unit-tests/service/test_response_parser.py +++ b/unit-tests/service/test_response_parser.py @@ -1,7 +1,22 @@ from unittest import TestCase -from keepercommander.service.util.parse_keeper_response import KeeperResponseParser +import json +from keepercommander.service.util.parse_keeper_response import KeeperResponseParser, ensure_record_add_json_format class TestKeeperResponseParser(TestCase): + def test_ensure_record_add_json_format(self): + self.assertEqual( + ensure_record_add_json_format('record-add -t test -rt login'), + 'record-add -t test -rt login --format=json', + ) + self.assertEqual( + ensure_record_add_json_format('record-add -t test --format=json'), + 'record-add -t test --format=json', + ) + self.assertEqual( + ensure_record_add_json_format('ls'), + 'ls', + ) + def test_parse_ls_command(self): """Test parsing of 'ls' command output""" sample_output = """# Folder UID Title Flags @@ -78,6 +93,36 @@ def test_parse_mkdir_command(self): result = KeeperResponseParser._parse_mkdir_command('Created folder with folder_uid=b4pBzT1WowoUXHk_US0SCg') self.assertEqual(result['data']['folder_uid'], 'b4pBzT1WowoUXHk_US0SCg') + def test_parse_record_add_command_two_line_output(self): + """record-add --self-destruct returns record_uid on line 1 and share_url on line 2""" + record_uid = 'riK9X5/XcxGPWRYM2Be1Ow==' + share_url = 'https://keepersecurity.com/vault/share#abc123' + response = f'{record_uid}\n{share_url}' + + result = KeeperResponseParser._parse_record_add_command(response) + self.assertEqual(result['data']['record_uid'], record_uid) + self.assertEqual(result['data']['share_url'], share_url) + + def test_parse_record_add_command_json_format(self): + """record-add --format=json is parsed as structured JSON""" + record_uid = 'riK9X5/XcxGPWRYM2Be1Ow==' + share_url = 'https://keepersecurity.com/vault/share#abc123' + response = json.dumps({'record_uid': record_uid, 'share_url': share_url}) + + result = KeeperResponseParser.parse_response( + 'record-add --self-destruct 2mi --format=json', + response, + ) + self.assertEqual(result['status'], 'success') + self.assertEqual(result['data']['record_uid'], record_uid) + self.assertEqual(result['data']['share_url'], share_url) + + def test_parse_record_add_command_uid_only(self): + """Normal record-add returns bare record_uid""" + record_uid = 'riK9X5/XcxGPWRYM2Be1Ow==' + result = KeeperResponseParser._parse_record_add_command(record_uid) + self.assertEqual(result['data']['record_uid'], record_uid) + def test_parse_get_command(self): """Test parsing of 'get' command output""" sample_output = """Title: Test Record diff --git a/unit-tests/test_command_record.py b/unit-tests/test_command_record.py index 03de045fc..cbef084b6 100644 --- a/unit-tests/test_command_record.py +++ b/unit-tests/test_command_record.py @@ -117,6 +117,51 @@ def artf(p, r, f): {"$ref": "script", "label": "rotationScripts"}, # real RT-definition label ] + def test_format_add_result(self): + record_uid = 'riK9X5/XcxGPWRYM2Be1Ow==' + share_url = 'https://keepersecurity.com/vault/share#abc123' + + self.assertEqual( + record_edit.RecordAddCommand._format_add_result(record_uid), + record_uid, + ) + self.assertEqual( + record_edit.RecordAddCommand._format_add_result(record_uid, share_url), + f'{record_uid}\n{share_url}', + ) + parsed = json.loads(record_edit.RecordAddCommand._format_add_result(record_uid, share_url, 'json')) + self.assertEqual(parsed['record_uid'], record_uid) + self.assertEqual(parsed['share_url'], share_url) + parsed = json.loads(record_edit.RecordAddCommand._format_add_result(record_uid, None, 'json')) + self.assertEqual(parsed, {'record_uid': record_uid}) + + def test_add_command_self_destruct_output(self): + params = get_synced_params() + cmd = record_edit.RecordAddCommand() + record_uid = utils.generate_uid() + share_url = 'https://keepersecurity.com/vault/share#abc123' + + with mock.patch('keepercommander.api.sync_down'), \ + mock.patch('keepercommander.record_management.add_record_to_folder') as ar, \ + mock.patch('keepercommander.api.communicate_rest'), \ + mock.patch('keepercommander.commands.record_edit.urlunparse', return_value=share_url): + def artf(p, r, f): + r.record_uid = record_uid + ar.side_effect = artf + + text_result = cmd.execute( + params, force=True, title='Test', record_type='login', + self_destruct='2mi', fields=['login=u', 'password=p'], + ) + self.assertEqual(text_result, f'{record_uid}\n{share_url}') + + json_result = json.loads(cmd.execute( + params, force=True, title='Test', record_type='login', + self_destruct='2mi', fields=['login=u', 'password=p'], format='json', + )) + self.assertEqual(json_result['record_uid'], record_uid) + self.assertEqual(json_result['share_url'], share_url) + def test_add_command_labels_default_is_legacy(self): # No --labels (and explicit --labels=on): fields with no label in the RT definition fall # back to the field type as the label; real definition labels are kept.