From 8b43a7d9e0838c2b16b1a4407a9b6a1107be6e2f Mon Sep 17 00:00:00 2001 From: adeshmukh-ks Date: Thu, 16 Jul 2026 20:15:48 +0530 Subject: [PATCH] Add NSF support to workflow commands --- keepercommander/commands/discoveryrotation.py | 53 ++++++++++--------- .../commands/workflow/approver_commands.py | 3 +- keepercommander/commands/workflow/helpers.py | 41 +++++++++----- unit-tests/pam/test_pam_rotation.py | 22 +++++++- 4 files changed, 76 insertions(+), 43 deletions(-) diff --git a/keepercommander/commands/discoveryrotation.py b/keepercommander/commands/discoveryrotation.py index a6fbb9cc9..fbc045a33 100644 --- a/keepercommander/commands/discoveryrotation.py +++ b/keepercommander/commands/discoveryrotation.py @@ -188,14 +188,30 @@ def parse_schedule_data(kwargs): def resolve_record_rotation_revision(params, record_uid): # type: (KeeperParams, str) -> int - """Return server-assigned rotation revision from cache, syncing when missing (not sequential).""" + """Return server-assigned rotation revision from cache, syncing when missing (not sequential). + + NSF rotations may not yet arrive via sync_down into ``record_rotation_cache``. + """ + cached = params.record_rotation_cache.get(record_uid) + if cached is not None and cached.get('revision') is not None: + return cached['revision'] + + nsf_records = getattr(params, 'nested_share_records', None) + if isinstance(nsf_records, dict): + nsf_rec = nsf_records.get(record_uid) or {} + if isinstance(nsf_rec, dict) and nsf_rec.get('revision') is not None: + return nsf_rec['revision'] + + api.sync_down(params) cached = params.record_rotation_cache.get(record_uid) - if cached is None or cached.get('revision') is None: - api.sync_down(params) - cached = params.record_rotation_cache.get(record_uid) - if cached is None or cached.get('revision') is None: - return 0 - return cached['revision'] + if cached is not None and cached.get('revision') is not None: + return cached['revision'] + + if isinstance(nsf_records, dict): + nsf_rec = nsf_records.get(record_uid) or {} + if isinstance(nsf_rec, dict) and nsf_rec.get('revision') is not None: + return nsf_rec['revision'] + return 0 def schedule_from_pam_config(record_pam_config): @@ -226,18 +242,6 @@ def resolve_record_schedule_data(schedule_data, current_record_rotation, schedul return schedule_from_pam_config(record_pam_config) -def resolve_record_rotation_revision(params, record_uid): - # type: (KeeperParams, str) -> int - """Return server-assigned rotation revision from cache, syncing when missing (not sequential).""" - cached = params.record_rotation_cache.get(record_uid) - if cached is None or cached.get('revision') is None: - api.sync_down(params) - cached = params.record_rotation_cache.get(record_uid) - if cached is None or cached.get('revision') is None: - return 0 - return cached['revision'] - - def refresh_vault_for_schedule_config(params): # type: (KeeperParams) -> None """Sync vault so PAM configuration defaultRotationSchedule is current for --schedule-config.""" @@ -1164,7 +1168,7 @@ def config_user(_dag, target_record, target_resource_uid, target_config_uid=None noop_rotation = True rq = router_pb2.RouterRecordRotationRequest() - rq.revision = current_record_rotation.get('revision', 0) + rq.revision = resolve_record_rotation_revision(params, target_record.record_uid) rq.recordUid = utils.base64_url_decode(target_record.record_uid) rq.configurationUid = utils.base64_url_decode(record_config_uid) rq.resourceUid = utils.base64_url_decode(record_resource_uid) if record_resource_uid else b'' @@ -1395,8 +1399,7 @@ def config_user(_dag, target_record, target_resource_uid, target_config_uid=None # 6. Construct Request object rq = router_pb2.RouterRecordRotationRequest() - if current_record_rotation: - rq.revision = current_record_rotation.get('revision', 0) + rq.revision = resolve_record_rotation_revision(params, target_record.record_uid) rq.recordUid = utils.base64_url_decode(target_record.record_uid) rq.configurationUid = utils.base64_url_decode(record_config_uid) rq.resourceUid = utils.base64_url_decode(record_resource_uid) if record_resource_uid else b'' @@ -1751,12 +1754,10 @@ def execute(self, params, **kwargs): row.append(f'{schedule_str}') # Controller Info - - enterprise_controllers_connected = router_get_connected_gateways(params) connected_controller = None - if enterprise_controllers_connected and controller_details: + if enterprise_controllers_connected_resp and controller_details: router_controllers = {controller.controllerUid: controller for controller in - list(enterprise_controllers_connected.controllers)} + list(enterprise_controllers_connected_resp.controllers)} connected_controller = router_controllers.get(controller_details.controllerUid) if connected_controller: diff --git a/keepercommander/commands/workflow/approver_commands.py b/keepercommander/commands/workflow/approver_commands.py index 644bc2296..ff0b5d796 100644 --- a/keepercommander/commands/workflow/approver_commands.py +++ b/keepercommander/commands/workflow/approver_commands.py @@ -105,7 +105,8 @@ def _extract_param(wf, key): def _decrypt_param(params, record_uid, encrypted_bytes): if not encrypted_bytes: return None - record_key = params.record_cache.get(record_uid, {}).get('record_key_unencrypted') + from ...nested_share_folder.common import get_record_key + record_key = get_record_key(params, record_uid, raise_on_missing=False) if not record_key: return 'No permission to view. Only users with record access can view this information.' try: diff --git a/keepercommander/commands/workflow/helpers.py b/keepercommander/commands/workflow/helpers.py index 2851c9c46..96e49c4fd 100644 --- a/keepercommander/commands/workflow/helpers.py +++ b/keepercommander/commands/workflow/helpers.py @@ -249,12 +249,15 @@ def submit_access_request(params: KeeperParams, record_uid: str, record = vault.KeeperRecord.load(params, record_uid) record_name = record.title if record else record_uid - record_key = params.record_cache.get(record_uid, {}).get('record_key_unencrypted') - if not record_key and (reason or ticket): - raise CommandError( - '', 'Record key not available — cannot encrypt reason/ticket. ' - 'You do not have sufficient access to this record to send encrypted parameters.', - ) + record_key = None + if reason or ticket: + from ...nested_share_folder.common import get_record_key + record_key = get_record_key(params, record_uid, raise_on_missing=False) + if not record_key: + raise CommandError( + '', 'Record key not available — cannot encrypt reason/ticket. ' + 'You do not have sufficient access to this record to send encrypted parameters.', + ) access_request = workflow_pb2.WorkflowAccessRequest() access_request.resource.CopyFrom(ProtobufRefBuilder.record_ref(record_uid_bytes, record_name)) @@ -302,12 +305,16 @@ class RecordResolver: @staticmethod def resolve(params, record_input, allow_missing=False): - if record_input in params.record_cache: - return record_input, vault.KeeperRecord.load(params, record_input) - for uid in params.record_cache: - rec = vault.KeeperRecord.load(params, uid) - if rec and rec.title == record_input: - return uid, rec + from ..pam.vault_target import resolve_pam_record + + if not record_input: + if allow_missing: + return None, None + raise CommandError('', 'Record is required') + + rec = resolve_pam_record(params, record_input) + if rec: + return rec.record_uid, rec if allow_missing: return None, None raise CommandError('', f'Record "{record_input}" not found') @@ -327,8 +334,10 @@ def validate_workflow_record_type(record): @staticmethod def get_uid_bytes(params: KeeperParams, record_uid: str) -> bytes: + from ..pam.vault_target import record_exists_in_vault + uid_bytes = utils.base64_url_decode(record_uid) - if record_uid not in params.record_cache: + if not record_exists_in_vault(params, record_uid): raise CommandError('', f'Record {record_uid} not found') return uid_bytes @@ -339,7 +348,11 @@ def resolve_name(params, resource_ref) -> str: if resource_ref.value: rec_uid = utils.base64_url_encode(resource_ref.value) rec = vault.KeeperRecord.load(params, rec_uid) - return rec.title if rec else '' + if rec: + return rec.title + from ..pam.vault_target import get_vault_record_title_type + title, _ = get_vault_record_title_type(params, rec_uid) + return title if title and title != '[record inaccessible]' else '' return '' @staticmethod diff --git a/unit-tests/pam/test_pam_rotation.py b/unit-tests/pam/test_pam_rotation.py index 60adc8a29..79f723e88 100644 --- a/unit-tests/pam/test_pam_rotation.py +++ b/unit-tests/pam/test_pam_rotation.py @@ -198,6 +198,7 @@ def test_execute_with_valid_record(self, mock_TunnelDAG, mock_load): self.assertTrue(mock_TunnelDAG.called) self.assertEqual(mock_typed_record.record_key, b'\x00' * 16) + @patch('keepercommander.commands.discoveryrotation.api.sync_down') @patch('keepercommander.commands.discoveryrotation.router_set_record_rotation_information') @patch('keepercommander.vault_extensions.find_records') @patch('keepercommander.commands.discoveryrotation.resolve_pam_record') @@ -207,13 +208,14 @@ def test_execute_with_valid_record(self, mock_TunnelDAG, mock_load): @patch('keepercommander.rest_api.SERVER_PUBLIC_KEYS', {8: ec.generate_private_key(ec.SECP256R1()).public_key()}) def test_execute_pam_user_with_config_no_resource(self, mock_TunnelDAG, mock_get_keeper_tokens, mock_load, mock_resolve_pam_record, mock_find_records, - mock_router_set_rotation): + mock_router_set_rotation, mock_sync_down): record_uid = 'OYNvVgpPPJBrVfYOIRtdag' config_uid = 'bU2LVM6LjX_hmCoSMDA7vg' resource_uid = 'dU2LVM6LjX_hmCoSMDA7vx' mock_params, mock_typed_record = create_mock_params_and_record() mock_params.record_rotation_cache = {} + mock_params.nested_share_records = {} mock_typed_record.record_uid = record_uid mock_load.return_value = mock_typed_record @@ -254,6 +256,7 @@ def test_execute_pam_user_with_config_no_resource(self, mock_TunnelDAG, mock_get mock_config_dag.link_resource_to_config.assert_called_once_with(resource_uid) mock_config_dag.link_user_to_resource.assert_called_once_with(record_uid, resource_uid, belongs_to=True) self.assertTrue(mock_router_set_rotation.called) + mock_sync_down.assert_called() config_call = mock_TunnelDAG.call_args_list[1] self.assertTrue(config_call.kwargs.get('is_config')) @@ -976,6 +979,7 @@ class TestResolveRecordRotationRevision(unittest.TestCase): def test_returns_cached_revision_without_sync(self): params = MagicMock() params.record_rotation_cache = {'record_uid': {'revision': 42}} + params.nested_share_records = {} with patch('keepercommander.commands.discoveryrotation.api.sync_down') as sync_down: revision = resolve_record_rotation_revision(params, 'record_uid') self.assertEqual(revision, 42) @@ -984,6 +988,7 @@ def test_returns_cached_revision_without_sync(self): def test_syncs_when_revision_missing_from_cache(self): params = MagicMock() params.record_rotation_cache = {} + params.nested_share_records = {} def _sync(p): params.record_rotation_cache['record_uid'] = {'revision': 17} @@ -995,21 +1000,32 @@ def _sync(p): def test_returns_zero_when_no_rotation_after_sync(self): params = MagicMock() params.record_rotation_cache = {} + params.nested_share_records = {} with patch('keepercommander.commands.discoveryrotation.api.sync_down'): revision = resolve_record_rotation_revision(params, 'record_uid') self.assertEqual(revision, 0) + def test_uses_nsf_record_revision_when_rotation_cache_empty(self): + params = MagicMock() + params.record_rotation_cache = {} + params.nested_share_records = {'nsf_uid': {'revision': 9, 'record_uid': 'nsf_uid'}} + with patch('keepercommander.commands.discoveryrotation.api.sync_down') as sync_down: + revision = resolve_record_rotation_revision(params, 'nsf_uid') + self.assertEqual(revision, 9) + sync_down.assert_not_called() + USER_UID = 'AAAAAAAAAAAAAAAAAAAAAA' CONFIG_UID = 'BBBBBBBBBBBBBBBBBBBBBB' CRON_SCHEDULE_JSON = '{"type":"CRON","cron":"0 0 3 ? * 2","tz":"Etc/UTC"}' -class TestResolveRecordRotationRevision(unittest.TestCase): +class TestResolveRecordRotationRevisionIam(unittest.TestCase): def test_returns_cached_revision_without_sync(self): params = MagicMock() params.record_rotation_cache = {USER_UID: {'revision': 42}} + params.nested_share_records = {} with patch('keepercommander.commands.discoveryrotation.api.sync_down') as sync_down: revision = resolve_record_rotation_revision(params, USER_UID) self.assertEqual(revision, 42) @@ -1018,6 +1034,7 @@ def test_returns_cached_revision_without_sync(self): def test_syncs_when_revision_missing_from_cache(self): params = MagicMock() params.record_rotation_cache = {} + params.nested_share_records = {} def _sync(p): params.record_rotation_cache[USER_UID] = {'revision': 17} @@ -1029,6 +1046,7 @@ def _sync(p): def test_returns_zero_when_no_rotation_after_sync(self): params = MagicMock() params.record_rotation_cache = {} + params.nested_share_records = {} with patch('keepercommander.commands.discoveryrotation.api.sync_down'): revision = resolve_record_rotation_revision(params, USER_UID) self.assertEqual(revision, 0)