From 561573807006b9dd2275c7febadb8640bb6ec87c Mon Sep 17 00:00:00 2001 From: sshrushanth-ks Date: Fri, 11 Sep 2026 11:01:08 +0530 Subject: [PATCH 1/4] Fix Gateway Name not displayed in pam rotation info output --- keepercommander/commands/discoveryrotation.py | 9 ++++- unit-tests/pam/test_pam_rotation.py | 34 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/keepercommander/commands/discoveryrotation.py b/keepercommander/commands/discoveryrotation.py index 9be7a7711..4bbb8b179 100644 --- a/keepercommander/commands/discoveryrotation.py +++ b/keepercommander/commands/discoveryrotation.py @@ -3265,7 +3265,14 @@ def execute(self, params, **kwargs): if rri_status_name == 'RRS_ONLINE': configuration_uid = utils.base64_url_encode(rri.configurationUid) - gateway_name = rri.controllerName if rri.controllerName else '-' + gateway_name = rri.controllerName + if not gateway_name and rri.controllerUid: + all_gateways = gateway_helper.get_all_gateways(params) + for gateway in all_gateways: + if gateway.controllerUid == rri.controllerUid: + gateway_name = gateway.controllerName + break + gateway_name = gateway_name if gateway_name else '-' gateway_uid = utils.base64_url_encode(rri.controllerUid) if rri.controllerUid else '-' def is_resource_ok(resource_id, params, configuration_uid): diff --git a/unit-tests/pam/test_pam_rotation.py b/unit-tests/pam/test_pam_rotation.py index 79f723e88..13a7e9b8d 100644 --- a/unit-tests/pam/test_pam_rotation.py +++ b/unit-tests/pam/test_pam_rotation.py @@ -860,6 +860,40 @@ def test_table_mode_returns_none(self, mock_rrg, mock_schedules): result = cmd.execute(mock_params, record_uid=record_uid, format='table') self.assertIsNone(result) + @patch('keepercommander.commands.discoveryrotation.gateway_helper.get_all_gateways') + @patch('keepercommander.commands.discoveryrotation.router_get_rotation_schedules') + @patch('keepercommander.commands.discoveryrotation.record_rotation_get') + def test_gateway_name_resolved_from_uid_when_empty(self, mock_rrg, mock_schedules, mock_get_gateways): + """When controllerName is empty, it should be resolved from gateway list using controllerUid.""" + from keeper_secrets_manager_core.utils import url_safe_str_to_bytes + from keepercommander import utils + record_uid = 'test_record_uid_' + record_uid_bytes = url_safe_str_to_bytes(record_uid) + + rri = self._make_rri('RRS_ONLINE') + rri.controllerName = '' + + mock_rrg.return_value = rri + + sched_mock = MagicMock() + sched_mock.schedules = [self._make_schedule(record_uid_bytes)] + mock_schedules.return_value = sched_mock + + mock_gateway = MagicMock() + mock_gateway.controllerUid = rri.controllerUid + mock_gateway.controllerName = 'gw-test-resolved' + mock_get_gateways.return_value = [mock_gateway] + + mock_params = create_mock_params() + mock_params.record_cache = {} + + cmd = PAMRouterGetRotationInfo() + result = cmd.execute(mock_params, record_uid=record_uid, format='json') + + self.assertIsNotNone(result, "Expected JSON string, got None") + data = json.loads(result) + self.assertEqual(data['gateway_name'], 'gw-test-resolved') + class TestUsesDefaultRotationSchedule(unittest.TestCase): From b92102c9d90677e983f4aa25c58892421063f193 Mon Sep 17 00:00:00 2001 From: sshrushanth-ks Date: Fri, 11 Sep 2026 13:38:38 +0530 Subject: [PATCH 2/4] Add UID type normalization and graceful error handling to gateway name resolution --- keepercommander/commands/discoveryrotation.py | 24 ++- unit-tests/pam/test_pam_rotation.py | 157 +++++++++++++++++- 2 files changed, 175 insertions(+), 6 deletions(-) diff --git a/keepercommander/commands/discoveryrotation.py b/keepercommander/commands/discoveryrotation.py index 4bbb8b179..6a2ffc394 100644 --- a/keepercommander/commands/discoveryrotation.py +++ b/keepercommander/commands/discoveryrotation.py @@ -3267,11 +3267,25 @@ def execute(self, params, **kwargs): configuration_uid = utils.base64_url_encode(rri.configurationUid) gateway_name = rri.controllerName if not gateway_name and rri.controllerUid: - all_gateways = gateway_helper.get_all_gateways(params) - for gateway in all_gateways: - if gateway.controllerUid == rri.controllerUid: - gateway_name = gateway.controllerName - break + def _normalize_uid(uid): + if uid is None: + return None + if isinstance(uid, (bytes, bytearray)): + return utils.base64_url_encode(uid) + return str(uid) + + target_uid = _normalize_uid(rri.controllerUid) + try: + all_gateways = gateway_helper.get_all_gateways(params) or [] + except Exception: + all_gateways = [] + + matched = next((g for g in all_gateways + if _normalize_uid(getattr(g, 'controllerUid', None)) == target_uid), None) + if matched: + gateway_name = getattr(matched, 'controllerName', None) + logging.debug(f"Resolved gateway name from controllerUid {target_uid} -> {gateway_name}") + gateway_name = gateway_name if gateway_name else '-' gateway_uid = utils.base64_url_encode(rri.controllerUid) if rri.controllerUid else '-' diff --git a/unit-tests/pam/test_pam_rotation.py b/unit-tests/pam/test_pam_rotation.py index 13a7e9b8d..1836b69df 100644 --- a/unit-tests/pam/test_pam_rotation.py +++ b/unit-tests/pam/test_pam_rotation.py @@ -866,7 +866,6 @@ def test_table_mode_returns_none(self, mock_rrg, mock_schedules): def test_gateway_name_resolved_from_uid_when_empty(self, mock_rrg, mock_schedules, mock_get_gateways): """When controllerName is empty, it should be resolved from gateway list using controllerUid.""" from keeper_secrets_manager_core.utils import url_safe_str_to_bytes - from keepercommander import utils record_uid = 'test_record_uid_' record_uid_bytes = url_safe_str_to_bytes(record_uid) @@ -894,6 +893,162 @@ def test_gateway_name_resolved_from_uid_when_empty(self, mock_rrg, mock_schedule data = json.loads(result) self.assertEqual(data['gateway_name'], 'gw-test-resolved') + @patch('keepercommander.commands.discoveryrotation.gateway_helper.get_all_gateways') + @patch('keepercommander.commands.discoveryrotation.router_get_rotation_schedules') + @patch('keepercommander.commands.discoveryrotation.record_rotation_get') + def test_gateway_name_present_unchanged(self, mock_rrg, mock_schedules, mock_get_gateways): + """When controllerName is present, it should be used without looking up gateways.""" + from keeper_secrets_manager_core.utils import url_safe_str_to_bytes + record_uid = 'test_record_uid_' + record_uid_bytes = url_safe_str_to_bytes(record_uid) + + rri = self._make_rri('RRS_ONLINE') + rri.controllerName = 'gw-original' + + mock_rrg.return_value = rri + + sched_mock = MagicMock() + sched_mock.schedules = [self._make_schedule(record_uid_bytes)] + mock_schedules.return_value = sched_mock + + mock_params = create_mock_params() + mock_params.record_cache = {} + + cmd = PAMRouterGetRotationInfo() + result = cmd.execute(mock_params, record_uid=record_uid, format='json') + + self.assertIsNotNone(result) + data = json.loads(result) + self.assertEqual(data['gateway_name'], 'gw-original') + mock_get_gateways.assert_not_called() + + @patch('keepercommander.commands.discoveryrotation.gateway_helper.get_all_gateways') + @patch('keepercommander.commands.discoveryrotation.router_get_rotation_schedules') + @patch('keepercommander.commands.discoveryrotation.record_rotation_get') + def test_gateway_name_empty_no_match_falls_back_to_dash(self, mock_rrg, mock_schedules, mock_get_gateways): + """When controllerName is empty and no gateway matches, should fall back to '-'.""" + from keeper_secrets_manager_core.utils import url_safe_str_to_bytes + record_uid = 'test_record_uid_' + record_uid_bytes = url_safe_str_to_bytes(record_uid) + + rri = self._make_rri('RRS_ONLINE') + rri.controllerName = '' + + mock_rrg.return_value = rri + + sched_mock = MagicMock() + sched_mock.schedules = [self._make_schedule(record_uid_bytes)] + mock_schedules.return_value = sched_mock + + mock_gateway = MagicMock() + mock_gateway.controllerUid = b'different_uid_' + mock_gateway.controllerName = 'gw-other' + mock_get_gateways.return_value = [mock_gateway] + + mock_params = create_mock_params() + mock_params.record_cache = {} + + cmd = PAMRouterGetRotationInfo() + result = cmd.execute(mock_params, record_uid=record_uid, format='json') + + self.assertIsNotNone(result) + data = json.loads(result) + self.assertEqual(data['gateway_name'], '-') + + @patch('keepercommander.commands.discoveryrotation.gateway_helper.get_all_gateways') + @patch('keepercommander.commands.discoveryrotation.router_get_rotation_schedules') + @patch('keepercommander.commands.discoveryrotation.record_rotation_get') + def test_gateway_name_resolved_with_different_uid_types(self, mock_rrg, mock_schedules, mock_get_gateways): + """When controllerUid types differ (bytes vs string), should still resolve correctly.""" + from keeper_secrets_manager_core.utils import url_safe_str_to_bytes + from keepercommander import utils + record_uid = 'test_record_uid_' + record_uid_bytes = url_safe_str_to_bytes(record_uid) + + rri = self._make_rri('RRS_ONLINE') + rri.controllerName = '' + + mock_rrg.return_value = rri + + sched_mock = MagicMock() + sched_mock.schedules = [self._make_schedule(record_uid_bytes)] + mock_schedules.return_value = sched_mock + + mock_gateway = MagicMock() + mock_gateway.controllerUid = utils.base64_url_encode(rri.controllerUid) + mock_gateway.controllerName = 'gw-test-resolved' + mock_get_gateways.return_value = [mock_gateway] + + mock_params = create_mock_params() + mock_params.record_cache = {} + + cmd = PAMRouterGetRotationInfo() + result = cmd.execute(mock_params, record_uid=record_uid, format='json') + + self.assertIsNotNone(result) + data = json.loads(result) + self.assertEqual(data['gateway_name'], 'gw-test-resolved') + + @patch('keepercommander.commands.discoveryrotation.gateway_helper.get_all_gateways') + @patch('keepercommander.commands.discoveryrotation.router_get_rotation_schedules') + @patch('keepercommander.commands.discoveryrotation.record_rotation_get') + def test_gateway_list_returns_none_falls_back_gracefully(self, mock_rrg, mock_schedules, mock_get_gateways): + """When get_all_gateways returns None, should fall back to '-' gracefully.""" + from keeper_secrets_manager_core.utils import url_safe_str_to_bytes + record_uid = 'test_record_uid_' + record_uid_bytes = url_safe_str_to_bytes(record_uid) + + rri = self._make_rri('RRS_ONLINE') + rri.controllerName = '' + + mock_rrg.return_value = rri + + sched_mock = MagicMock() + sched_mock.schedules = [self._make_schedule(record_uid_bytes)] + mock_schedules.return_value = sched_mock + + mock_get_gateways.return_value = None + + mock_params = create_mock_params() + mock_params.record_cache = {} + + cmd = PAMRouterGetRotationInfo() + result = cmd.execute(mock_params, record_uid=record_uid, format='json') + + self.assertIsNotNone(result) + data = json.loads(result) + self.assertEqual(data['gateway_name'], '-') + + @patch('keepercommander.commands.discoveryrotation.gateway_helper.get_all_gateways') + @patch('keepercommander.commands.discoveryrotation.router_get_rotation_schedules') + @patch('keepercommander.commands.discoveryrotation.record_rotation_get') + def test_gateway_list_raises_exception_falls_back_gracefully(self, mock_rrg, mock_schedules, mock_get_gateways): + """When get_all_gateways raises an exception, should fall back to '-' without crashing.""" + from keeper_secrets_manager_core.utils import url_safe_str_to_bytes + record_uid = 'test_record_uid_' + record_uid_bytes = url_safe_str_to_bytes(record_uid) + + rri = self._make_rri('RRS_ONLINE') + rri.controllerName = '' + + mock_rrg.return_value = rri + + sched_mock = MagicMock() + sched_mock.schedules = [self._make_schedule(record_uid_bytes)] + mock_schedules.return_value = sched_mock + + mock_get_gateways.side_effect = RuntimeError("Gateway service unavailable") + + mock_params = create_mock_params() + mock_params.record_cache = {} + + cmd = PAMRouterGetRotationInfo() + result = cmd.execute(mock_params, record_uid=record_uid, format='json') + + self.assertIsNotNone(result) + data = json.loads(result) + self.assertEqual(data['gateway_name'], '-') + class TestUsesDefaultRotationSchedule(unittest.TestCase): From ac46e9f5f678b466753e719562bb00a9c1a91b66 Mon Sep 17 00:00:00 2001 From: sshrushanth-ks Date: Wed, 16 Sep 2026 16:32:33 +0530 Subject: [PATCH 3/4] Improve gateway name resolution: add UID validation and specific exception logging --- keepercommander/commands/discoveryrotation.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/keepercommander/commands/discoveryrotation.py b/keepercommander/commands/discoveryrotation.py index 6a2ffc394..de68e9c26 100644 --- a/keepercommander/commands/discoveryrotation.py +++ b/keepercommander/commands/discoveryrotation.py @@ -3277,14 +3277,17 @@ def _normalize_uid(uid): target_uid = _normalize_uid(rri.controllerUid) try: all_gateways = gateway_helper.get_all_gateways(params) or [] - except Exception: + except (Exception,) as ex: + logging.debug(f"Failed to retrieve gateway list for name resolution: {ex}") all_gateways = [] - matched = next((g for g in all_gateways - if _normalize_uid(getattr(g, 'controllerUid', None)) == target_uid), None) - if matched: - gateway_name = getattr(matched, 'controllerName', None) - logging.debug(f"Resolved gateway name from controllerUid {target_uid} -> {gateway_name}") + if all_gateways and target_uid: + matched = next((g for g in all_gateways + if _normalize_uid(getattr(g, 'controllerUid', None)) == target_uid), None) + if matched: + gateway_name = getattr(matched, 'controllerName', None) + if gateway_name: + logging.debug(f"Resolved gateway name from controllerUid {target_uid} -> {gateway_name}") gateway_name = gateway_name if gateway_name else '-' gateway_uid = utils.base64_url_encode(rri.controllerUid) if rri.controllerUid else '-' From e4bdb62c248ce905068774174f0f8c4b7c12a99e Mon Sep 17 00:00:00 2001 From: sshrushanth-ks Date: Wed, 16 Sep 2026 16:45:50 +0530 Subject: [PATCH 4/4] Prevent None == None false positive by explicitly checking target_uid before gateway lookup --- keepercommander/commands/discoveryrotation.py | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/keepercommander/commands/discoveryrotation.py b/keepercommander/commands/discoveryrotation.py index de68e9c26..2360956a4 100644 --- a/keepercommander/commands/discoveryrotation.py +++ b/keepercommander/commands/discoveryrotation.py @@ -3275,19 +3275,22 @@ def _normalize_uid(uid): return str(uid) target_uid = _normalize_uid(rri.controllerUid) - try: - all_gateways = gateway_helper.get_all_gateways(params) or [] - except (Exception,) as ex: - logging.debug(f"Failed to retrieve gateway list for name resolution: {ex}") - all_gateways = [] - - if all_gateways and target_uid: - matched = next((g for g in all_gateways - if _normalize_uid(getattr(g, 'controllerUid', None)) == target_uid), None) - if matched: - gateway_name = getattr(matched, 'controllerName', None) - if gateway_name: - logging.debug(f"Resolved gateway name from controllerUid {target_uid} -> {gateway_name}") + if target_uid is None: + gateway_name = None + else: + try: + all_gateways = gateway_helper.get_all_gateways(params) or [] + except (Exception,) as ex: + logging.debug(f"Failed to retrieve gateway list for name resolution: {ex}") + all_gateways = [] + + if all_gateways: + matched = next((g for g in all_gateways + if _normalize_uid(getattr(g, 'controllerUid', None)) == target_uid), None) + if matched: + gateway_name = getattr(matched, 'controllerName', None) + if gateway_name: + logging.debug(f"Resolved gateway name from controllerUid {target_uid} -> {gateway_name}") gateway_name = gateway_name if gateway_name else '-' gateway_uid = utils.base64_url_encode(rri.controllerUid) if rri.controllerUid else '-'