From b384cf6827eb603dbc07d3fdfd39c5faaa18c77a Mon Sep 17 00:00:00 2001 From: idimov-keeper <78815270+idimov-keeper@users.noreply.github.com> Date: Fri, 5 Jun 2026 19:10:04 -0500 Subject: [PATCH 1/8] Fix kcm-import --output file permissions on Windows (#2130) * Fix kcm-import --output file permissions on Windows via utils.set_file_permissions * Fix flaky set_record_rotation body test: decrypt round-trip instead of first-byte check --- .../commands/pam_import/kcm_import.py | 1 + .../test_dag_layer_b_set_record_rotation.py | 18 ++++++++-- unit-tests/pam/test_kcm_import.py | 33 +++++++++++++------ 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/keepercommander/commands/pam_import/kcm_import.py b/keepercommander/commands/pam_import/kcm_import.py index 5bc4e5f6e..bb2439003 100644 --- a/keepercommander/commands/pam_import/kcm_import.py +++ b/keepercommander/commands/pam_import/kcm_import.py @@ -1367,6 +1367,7 @@ def execute(self, params, **kwargs): fd = os.open(output_file, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) with os.fdopen(fd, 'w') as f: json.dump(out_data, f, indent=2) + utils.set_file_permissions(output_file) redact_note = '' if include_creds else ' (credentials redacted)' logging.warning('JSON written to %s (%d resources, %d users)%s', output_file, num_resources, num_users, redact_note) diff --git a/unit-tests/pam/test_dag_layer_b_set_record_rotation.py b/unit-tests/pam/test_dag_layer_b_set_record_rotation.py index 19495e8b0..334cc0906 100644 --- a/unit-tests/pam/test_dag_layer_b_set_record_rotation.py +++ b/unit-tests/pam/test_dag_layer_b_set_record_rotation.py @@ -83,18 +83,30 @@ def test_set_record_rotation_hits_correct_url(): def test_set_record_rotation_sends_protobuf_body(): from keepercommander.commands.pam.router_helper import router_set_record_rotation_information + from keepercommander import crypto, utils rq = router_pb2.RouterRecordRotationRequest( recordUid=RECORD_UID, configurationUid=CONFIG_UID, resourceUid=RESOURCE_UID, schedule='0 0 * * *', ) + transmission_key = utils.generate_aes_key() with patch(REQUESTS_TARGET, return_value=_ok_router_response()) as mock_req: - router_set_record_rotation_information(_mock_params(), rq) + router_set_record_rotation_information(_mock_params(), rq, transmission_key=transmission_key) body = mock_req.call_args.kwargs.get('data') assert isinstance(body, (bytes, bytearray)) - # Encrypted blob, not JSON - assert not body.startswith(b'{') + # Body is the AES-GCM-encrypted protobuf — not the plaintext proto, and not + # JSON. Decrypt with the known transmission key and confirm it round-trips. + # (A first-byte heuristic like `not body.startswith(b'{')` is flaky: ~1/256 + # of ciphertexts legitimately start with 0x7b.) + assert body != rq.SerializeToString() + decrypted = crypto.decrypt_aes_v2(body, transmission_key) + parsed = router_pb2.RouterRecordRotationRequest() + parsed.ParseFromString(decrypted) + assert parsed.recordUid == RECORD_UID + assert parsed.configurationUid == CONFIG_UID + assert parsed.resourceUid == RESOURCE_UID + assert parsed.schedule == '0 0 * * *' # --------------------------------------------------------------------------- # diff --git a/unit-tests/pam/test_kcm_import.py b/unit-tests/pam/test_kcm_import.py index e8b5db5ad..147c7227f 100644 --- a/unit-tests/pam/test_kcm_import.py +++ b/unit-tests/pam/test_kcm_import.py @@ -11,6 +11,8 @@ import json import os +import platform +import subprocess import sys import tempfile import unittest @@ -516,9 +518,8 @@ def test_output_file_pipeline(self, mock_getpass, MockConnector): cmd = PAMProjectKCMImportCommand() params = MagicMock() - import tempfile - with tempfile.NamedTemporaryFile(suffix='.json', delete=False) as f: - output_path = f.name + tmp_dir = tempfile.mkdtemp() + output_path = os.path.join(tmp_dir, 'output.json') try: # With --include-credentials, passwords are preserved @@ -547,6 +548,7 @@ def test_output_file_pipeline(self, mock_getpass, MockConnector): finally: if os.path.exists(output_path): os.unlink(output_path) + os.rmdir(tmp_dir) @patch('keepercommander.commands.pam_import.kcm_import.KCMDatabaseConnector') @patch('keepercommander.commands.pam_import.kcm_import.getpass.getpass', @@ -562,9 +564,8 @@ def test_output_file_redacted_by_default(self, mock_getpass, MockConnector): cmd = PAMProjectKCMImportCommand() params = MagicMock() - import tempfile - with tempfile.NamedTemporaryFile(suffix='.json', delete=False) as f: - output_path = f.name + tmp_dir = tempfile.mkdtemp() + output_path = os.path.join(tmp_dir, 'output.json') try: cmd.execute(params, @@ -588,6 +589,7 @@ def test_output_file_redacted_by_default(self, mock_getpass, MockConnector): finally: if os.path.exists(output_path): os.unlink(output_path) + os.rmdir(tmp_dir) @patch('keepercommander.commands.pam_import.kcm_import.KCMDatabaseConnector') @patch('keepercommander.commands.pam_import.kcm_import.getpass.getpass', @@ -1290,7 +1292,6 @@ def test_output_file_owner_only(self, mock_getpass, MockConnector): cmd = PAMProjectKCMImportCommand() params = MagicMock() - import stat tmp_dir = tempfile.mkdtemp() output_path = os.path.join(tmp_dir, 'test_output.json') try: @@ -1298,9 +1299,21 @@ def test_output_file_owner_only(self, mock_getpass, MockConnector): db_host='127.0.0.1', output=output_path) - file_mode = os.stat(output_path).st_mode & 0o777 - self.assertEqual(file_mode, 0o600, - f'Expected 0o600, got {oct(file_mode)}') + # POSIX: verifies mode == 0o600 via os.stat. + # Windows: os.stat().st_mode reads DOS attributes, not the NTFS + # DACL, so verify the icacls-applied ACL has the principals + # stripped by utils.set_file_permissions (SYSTEM, Administrators). + if platform.system() == 'Windows': + acl = subprocess.run(['icacls', output_path], + capture_output=True, text=True, check=True).stdout + self.assertNotIn('NT AUTHORITY\\SYSTEM', acl, + f'SYSTEM should have been removed from ACL:\n{acl}') + self.assertNotIn('BUILTIN\\Administrators', acl, + f'Administrators should have been removed from ACL:\n{acl}') + else: + file_mode = os.stat(output_path).st_mode & 0o777 + self.assertEqual(file_mode, 0o600, + f'Expected 0o600, got {oct(file_mode)}') finally: if os.path.exists(output_path): os.unlink(output_path) From 911ae245526d4a7a8f4651e81d59c55feadd21a9 Mon Sep 17 00:00:00 2001 From: Sergey Kolupaev Date: Sat, 6 Jun 2026 21:05:57 -0700 Subject: [PATCH 2/8] Release 18.0.7 --- keepercommander/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keepercommander/__init__.py b/keepercommander/__init__.py index f16f8c4d4..845f8bb94 100644 --- a/keepercommander/__init__.py +++ b/keepercommander/__init__.py @@ -10,4 +10,4 @@ # Contact: commander@keepersecurity.com # -__version__ = '18.0.6' +__version__ = '18.0.7' From 49fe213d05106a4310eda28b5f53fec21684002a Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Mon, 8 Jun 2026 21:03:44 +0530 Subject: [PATCH 3/8] Handle docker persistent login error and keeper get lookup (#2121) (#2133) * Add error handling for persistent login break in docker and strengthen keeper get lookup from shell * Fix unit tests --- keepercommander/auth/console_ui.py | 40 ++++++++++++++++++++++ keepercommander/commands/helpers/record.py | 17 +++++++++ keepercommander/commands/record.py | 3 ++ unit-tests/test_command_record.py | 8 +++++ 4 files changed, 68 insertions(+) diff --git a/keepercommander/auth/console_ui.py b/keepercommander/auth/console_ui.py index 0a60b077b..eca6bf3b4 100644 --- a/keepercommander/auth/console_ui.py +++ b/keepercommander/auth/console_ui.py @@ -18,6 +18,31 @@ def _stderr(msg=''): print(msg, file=sys.stderr) +_HEADLESS_AUTH_MSG_SHOWN = False + + +def _is_interactive(): + try: + return bool(sys.stdin) and sys.stdin.isatty() + except Exception: + return False + + +def _fail_headless_auth(step): + """In headless/service mode, persistent login often needs a follow-up prompt + (password, SSO, 2FA, device approval) that cannot be answered. Log once and + cancel so the caller exits cleanly instead of looping or spamming getpass.""" + global _HEADLESS_AUTH_MSG_SHOWN + if not _HEADLESS_AUTH_MSG_SHOWN: + _HEADLESS_AUTH_MSG_SHOWN = True + logging.error( + 'Persistent login is not working in this non-interactive environment ' + '(possibly due to an IP/location change). ' + 'Re-run Commander/Docker setup from this network, then restart the service.' + ) + step.cancel() + + class ConsoleLoginUi(login_steps.LoginUi): def __init__(self): self._show_device_approval_help = True @@ -28,6 +53,9 @@ def __init__(self): self._failed_password_attempt = 0 def on_device_approval(self, step): + if not _is_interactive(): + _fail_headless_auth(step) + return if self._show_device_approval_help: _stderr(f"\n{Fore.YELLOW}Device Approval Required{Fore.RESET}\n") _stderr(f"{Fore.CYAN}Select an approval method:{Fore.RESET}") @@ -123,6 +151,9 @@ def two_factor_channel_to_desc(channel): # type: (login_steps.TwoFactorChannel return 'Backup Codes' def on_two_factor(self, step): + if not _is_interactive(): + _fail_headless_auth(step) + return channels = step.get_channels() if self._show_two_factor_help: @@ -273,6 +304,9 @@ def on_two_factor(self, step): logging.warning(f'{Fore.YELLOW}Invalid 2FA code. Please try again.{Fore.RESET}') def on_password(self, step): + if not _is_interactive(): + _fail_headless_auth(step) + return if self._show_password_help: _stderr(f'{Fore.CYAN}Enter master password for {Fore.WHITE}{step.username}{Fore.RESET}') @@ -293,6 +327,9 @@ def on_password(self, step): step.cancel() def on_sso_redirect(self, step): + if not _is_interactive(): + _fail_headless_auth(step) + return try: wb = webbrowser.get() wrappers = set('xdg-open|gvfs-open|gnome-open|x-www-browser|www-browser'.split('|')) @@ -360,6 +397,9 @@ def on_sso_redirect(self, step): break def on_sso_data_key(self, step): + if not _is_interactive(): + _fail_headless_auth(step) + return if self._show_sso_data_key_help: _stderr(f'\n{Fore.YELLOW}Device Approval Required for SSO{Fore.RESET}\n') _stderr(f'{Fore.CYAN}Select an approval method:{Fore.RESET}') diff --git a/keepercommander/commands/helpers/record.py b/keepercommander/commands/helpers/record.py index 7769f5f7e..a2f97976f 100644 --- a/keepercommander/commands/helpers/record.py +++ b/keepercommander/commands/helpers/record.py @@ -1,9 +1,26 @@ +import re from typing import Set, Optional from ... import api +from ...error import CommandError from ...params import KeeperParams from ...subfolder import try_resolve_path +# Block shell chaining markers in `get` lookup tokens. +_GET_LOOKUP_CONTROL_CHARS_RE = re.compile(r'[\r\n\x00]') +_GET_LOOKUP_SHELL_METACHAR_RE = re.compile(r'[;|]') +_GET_LOOKUP_CHAIN_RE = re.compile(r'&&') + + +def raise_if_unsafe_get_lookup_token(token, command='get'): + # type: (str, str) -> None + if not token: + raise CommandError(command, 'Invalid record identifier: forbidden characters') + if (_GET_LOOKUP_CONTROL_CHARS_RE.search(token) + or _GET_LOOKUP_SHELL_METACHAR_RE.search(token) + or _GET_LOOKUP_CHAIN_RE.search(token)): + raise CommandError(command, 'Invalid record identifier: forbidden characters') + # Get record UID(s) given one of its identifiers: name (if current folder contains the record), path, or UID def get_record_uids(params, name): # type: (KeeperParams, str) -> Set[Optional[str]] diff --git a/keepercommander/commands/record.py b/keepercommander/commands/record.py index 0c8d152de..99355974c 100644 --- a/keepercommander/commands/record.py +++ b/keepercommander/commands/record.py @@ -20,6 +20,7 @@ import re from functools import reduce from typing import Dict, Any, List, Optional, Iterable, Tuple, Set +from .helpers.record import raise_if_unsafe_get_lookup_token from colorama import Fore, Back, Style @@ -294,6 +295,8 @@ def execute(self, params, **kwargs): if not uid: raise CommandError('get', 'UID parameter is required') + raise_if_unsafe_get_lookup_token(uid) + fmt = kwargs.get('format') or 'detail' # First try to interpret as UID diff --git a/unit-tests/test_command_record.py b/unit-tests/test_command_record.py index fc61833c9..ebfeb2f8f 100644 --- a/unit-tests/test_command_record.py +++ b/unit-tests/test_command_record.py @@ -248,6 +248,14 @@ def test_get_invalid_uid(self): with self.assertRaises(CommandError): cmd.execute(params, uid='invalid') + def test_get_rejects_shell_metacharacters_in_lookup_token(self): + params = get_synced_params() + cmd = record.RecordGetUidCommand() + + with self.assertRaises(CommandError) as context: + cmd.execute(params, uid='x;cd $HOME && id > pwned_keeper_rce.txt;#"unclosed') + self.assertIn('forbidden characters', context.exception.message) + def test_append_notes_command(self): params = get_synced_params() cmd = record_edit.RecordAppendNotesCommand() From 384d12b9eca5f56a6c91cad1666117323f5a0090 Mon Sep 17 00:00:00 2001 From: John Walstra Date: Mon, 8 Jun 2026 10:11:02 -0500 Subject: [PATCH 4/8] fix: sync keeper-dag and discovery-common golden repo. --- .../discovery_common/__version__.py | 2 +- .../discovery_common/infrastructure.py | 6 +- keepercommander/discovery_common/process.py | 6 +- keepercommander/discovery_common/rm_types.py | 2 +- keepercommander/discovery_common/types.py | 2 +- keepercommander/discovery_common/verify.py | 2 +- keepercommander/keeper_dag/__version__.py | 2 +- .../keeper_dag/connection/__init__.py | 151 +++++++++++++++++- keepercommander/keeper_dag/connection/ksm.py | 1 + .../keeper_dag/connection/local.py | 35 ++++ keepercommander/keeper_dag/dag.py | 87 +++++++++- keepercommander/keeper_dag/struct/__init__.py | 31 ++++ keepercommander/keeper_dag/struct/default.py | 63 +++++++- keepercommander/keeper_dag/struct/protobuf.py | 77 +++++++-- keepercommander/keeper_dag/types.py | 19 ++- keepercommander/keeper_dag/utils.py | 4 +- 16 files changed, 450 insertions(+), 40 deletions(-) diff --git a/keepercommander/discovery_common/__version__.py b/keepercommander/discovery_common/__version__.py index 87bb711d7..7025f5029 100644 --- a/keepercommander/discovery_common/__version__.py +++ b/keepercommander/discovery_common/__version__.py @@ -1 +1 @@ -__version__ = '1.1.14' +__version__ = '1.1.15' diff --git a/keepercommander/discovery_common/infrastructure.py b/keepercommander/discovery_common/infrastructure.py index 744c7951b..ee2675955 100644 --- a/keepercommander/discovery_common/infrastructure.py +++ b/keepercommander/discovery_common/infrastructure.py @@ -394,7 +394,9 @@ def to_dot(self, graph_format: str = "svg", show_hex_uid: bool = False, head_uids.append(edge.head_uid) def _render_edge(e): - + # _render_edge is invoked immediately within the loop below, so capturing the + # loop variables v/content is safe. + # pylint: disable=cell-var-from-loop edge_color = "grey" style = "solid" @@ -439,7 +441,7 @@ def _render_edge(e): tooltip=edge_tip) for head_uid in head_uids: - version, edge = v.get_highest_edge_version(head_uid) + _, edge = v.get_highest_edge_version(head_uid) _render_edge(edge) data_edge = v.get_data() diff --git a/keepercommander/discovery_common/process.py b/keepercommander/discovery_common/process.py index 836243c41..a99ee2b06 100644 --- a/keepercommander/discovery_common/process.py +++ b/keepercommander/discovery_common/process.py @@ -377,7 +377,7 @@ def _directory_exists(self, domain: str, directory_info_func: Callable, context: for provider_vertex in provider_vertices: content = DiscoveryObject.get_discovery_object(provider_vertex) found = False - for domain in domains: + for domain in domains: # pylint: disable=redefined-argument-from-local for provider_domain in content.item.info.get("domains", []): if domain.lower() in provider_domain.lower(): found = True @@ -453,7 +453,7 @@ def _find_directory_user(self, found_vertex = None if find_user is not None: - user, domain = split_user_and_domain(find_user) + user, _ = split_user_and_domain(find_user) if user_content.item.user.lower() == user.lower(): found_vertex = user_vertex elif user_content.item.user.lower() == find_user.lower(): @@ -1185,7 +1185,7 @@ def _process_admin_user(self, # We need to populate the id and uid of the content, now that we have data in the content. self.populate_admin_content_ids(admin_content, resource_vertex) - ad_user, ad_domain = split_user_and_domain(admin_content.item.user) + _, ad_domain = split_user_and_domain(admin_content.item.user) if ad_domain is not None and admin_content.item.source == LOCAL_USER: self.logger.debug("The admin is an directory user, but the source is set to a local user") diff --git a/keepercommander/discovery_common/rm_types.py b/keepercommander/discovery_common/rm_types.py index a647ca933..1aa513d88 100644 --- a/keepercommander/discovery_common/rm_types.py +++ b/keepercommander/discovery_common/rm_types.py @@ -465,7 +465,7 @@ class RmOracleUserAddMeta(RmMetaBase): class RmOracleRoleAddMeta(RmMetaBase): - not_identified: bool = False, + not_identified: bool = False identified_by_password: Optional[str] = None identified_using: Optional[str] = None identified_externally: bool = False diff --git a/keepercommander/discovery_common/types.py b/keepercommander/discovery_common/types.py index 5a79a9115..6942e87eb 100644 --- a/keepercommander/discovery_common/types.py +++ b/keepercommander/discovery_common/types.py @@ -736,7 +736,7 @@ def has_dn(self, user) -> bool: return False - + class PromptResult(BaseModel): # "add" and "ignore" are the only action diff --git a/keepercommander/discovery_common/verify.py b/keepercommander/discovery_common/verify.py index 5e90ab5a7..dd6e6a3fd 100644 --- a/keepercommander/discovery_common/verify.py +++ b/keepercommander/discovery_common/verify.py @@ -403,7 +403,7 @@ def _check(vertex: DAGVertex, indent: int = 0): # Get all the child vertices, allow self ref, so we can delete it if not already deleted. for next_vertex in vertex.has_vertices(allow_self_ref=True): if next_vertex.uid == vertex.uid: - version, edge = next_vertex.get_highest_edge_version(vertex.uid) + _, edge = next_vertex.get_highest_edge_version(vertex.uid) if edge.edge_type == EdgeType.DELETION: continue else: diff --git a/keepercommander/keeper_dag/__version__.py b/keepercommander/keeper_dag/__version__.py index 874042f3b..ed7133b36 100644 --- a/keepercommander/keeper_dag/__version__.py +++ b/keepercommander/keeper_dag/__version__.py @@ -1 +1 @@ -__version__ = '1.1.10' # pragma: no cover +__version__ = '1.1.11' # pragma: no cover diff --git a/keepercommander/keeper_dag/connection/__init__.py b/keepercommander/keeper_dag/connection/__init__.py index 762acde53..24ef3ba66 100644 --- a/keepercommander/keeper_dag/connection/__init__.py +++ b/keepercommander/keeper_dag/connection/__init__.py @@ -31,6 +31,8 @@ class ConnectionBase: ADD_DATA = "/add_data" SYNC = "/sync" + MULTI_SYNC = "/multi_sync" + GET_LEAFS = "/get_leafs" TIMEOUT = 30 @@ -59,7 +61,7 @@ def __init__(self, if self.log_transactions_dir is None: self.log_transactions_dir = "." - if self.log_transactions is True: + if self.log_transactions: self.logger.info("keeper-dag transaction logging is ENABLED; " f"write directory at {self.log_transactions_dir}") @@ -99,8 +101,11 @@ def get_encrypted_payload_data(encrypted_payload_data: bytes) -> bytes: @staticmethod def get_router_host(server_hostname: str): - if server_hostname and '://' in server_hostname: # accept URL-formatted inputs + # Defensive: accept URL-formatted inputs (e.g. "https://keepersecurity.com") + # and extract the bare hostname before the GovCloud subdomain check. + if server_hostname and '://' in server_hostname: server_hostname = server_hostname.split('://', 1)[1].split('/', 1)[0] + # Only PROD GovCloud strips the subdomain (workaround for prod infrastructure). # DEV/QA GOV (govcloud.dev.keepersecurity.us, govcloud.qa.keepersecurity.us) keep govcloud. if server_hostname == 'govcloud.keepersecurity.us': @@ -222,10 +227,10 @@ def sync(self, sync_query: Union[SyncQuery, gs_pb2.GraphSyncQuery], graph_id: Optional[int] = None, endpoint: Optional[str] = None, - agent: Optional[str] = None) -> bytes: + agent: Optional[str] = None) -> Optional[bytes]: if agent is None: - f"keeper-dag/{__version__}" + agent = f"keeper-dag/{__version__}" endpoint = self._endpoint(ConnectionBase.SYNC, endpoint) self.logger.debug(f"endpoint {endpoint}") @@ -238,13 +243,13 @@ def sync(self, headers=headers, payload=sync_query) - if self.use_read_protobuf: + if payload is not None and self.use_read_protobuf: try: self.logger.debug(f"decrypt payload with transmission key {kotlin_bytes(self.transmission_key)}") payload = self.get_encrypted_payload_data(payload) payload = decrypt_aes(payload, self.transmission_key) except Exception as err: - self.logger.error(f"Could not decrypt protobuf graph sync response: {type(err)}, {err}") + self.logger.error(f"Could not decrypt protobuf graph sync response: {err}") self.write_transaction_log( graph_id=graph_id, @@ -290,7 +295,7 @@ def add_data(self, agent: Optional[str] = None): if agent is None: - f"keeper-dag/{__version__}" + agent = f"keeper-dag/{__version__}" endpoint = self._endpoint(ConnectionBase.ADD_DATA, endpoint) self.logger.debug(f"endpoint {endpoint}") @@ -331,3 +336,135 @@ def add_data(self, error=str(err) ) raise DAGException(f"Could not create a new DAG structure: {err}") + + def multi_sync(self, + multi_query: Union[BaseModel, gs_pb2.GraphSyncMultiQuery], + graph_id: Optional[int] = None, + endpoint: Optional[str] = None, + agent: Optional[str] = None) -> bytes: + """POST a GraphSyncMultiQuery to /multi_sync. + + Used by per-graph reads: after `get_leafs` discovers the stream refs + rooted at the graph's origin, `multi_sync` fetches sync data for all + those streams in one round-trip. Mirrors `sync()` in transport shape + (encrypt/headers, decrypt-on-read, transaction log, error handling). + """ + if agent is None: + agent = f"keeper-dag/{__version__}" + + endpoint = self._endpoint(ConnectionBase.MULTI_SYNC, endpoint) + self.logger.debug(f"endpoint {endpoint}") + + try: + multi_query, headers = self.payload_and_headers(multi_query) + payload = self.rest_call_to_router(http_method="POST", + endpoint=endpoint, + agent=agent, + headers=headers, + payload=multi_query) + + if self.use_read_protobuf: + try: + self.logger.debug(f"decrypt payload with transmission key {kotlin_bytes(self.transmission_key)}") + payload = self.get_encrypted_payload_data(payload) + payload = decrypt_aes(payload, self.transmission_key) + except Exception as err: + self.logger.error(f"Could not decrypt protobuf graph multi-sync response: {type(err)}, {err}") + + self.write_transaction_log( + graph_id=graph_id, + request=multi_query, + response=payload, + agent=agent, + endpoint=endpoint, + error=None + ) + + return payload + + except DAGConnectionException as err: + self.write_transaction_log( + graph_id=graph_id, + request=multi_query, + response=None, + agent=agent, + endpoint=endpoint, + error=str(err) + ) + raise err + except Exception as err: + self.write_transaction_log( + graph_id=graph_id, + request=multi_query, + response=None, + agent=agent, + endpoint=endpoint, + error=str(err) + ) + raise DAGException(f"Could not load the DAG structure (multi_sync): {err}") + + def get_leafs(self, + leafs_query: Union[BaseModel, gs_pb2.GraphSyncLeafsQuery], + graph_id: Optional[int] = None, + endpoint: Optional[str] = None, + agent: Optional[str] = None) -> bytes: + """POST a GraphSyncLeafsQuery to /get_leafs. + + Returns the serialized GraphSyncRefsResult — the list of stream refs + rooted at the queried vertices. Used as the discovery step before a + `multi_sync` call (per the per-graph read pattern that Web Vault + already uses). + """ + if agent is None: + agent = f"keeper-dag/{__version__}" + + endpoint = self._endpoint(ConnectionBase.GET_LEAFS, endpoint) + self.logger.debug(f"endpoint {endpoint}") + + try: + leafs_query, headers = self.payload_and_headers(leafs_query) + payload = self.rest_call_to_router(http_method="POST", + endpoint=endpoint, + agent=agent, + headers=headers, + payload=leafs_query) + + if self.use_read_protobuf: + try: + self.logger.debug(f"decrypt payload with transmission key {kotlin_bytes(self.transmission_key)}") + payload = self.get_encrypted_payload_data(payload) + payload = decrypt_aes(payload, self.transmission_key) + except Exception as err: + self.logger.error(f"Could not decrypt protobuf get_leafs response: {type(err)}, {err}") + + self.write_transaction_log( + graph_id=graph_id, + request=leafs_query, + response=payload, + agent=agent, + endpoint=endpoint, + error=None + ) + + return payload + + except DAGConnectionException as err: + self.write_transaction_log( + graph_id=graph_id, + request=leafs_query, + response=None, + agent=agent, + endpoint=endpoint, + error=str(err) + ) + raise err + except Exception as err: + self.write_transaction_log( + graph_id=graph_id, + request=leafs_query, + response=None, + agent=agent, + endpoint=endpoint, + error=str(err) + ) + raise DAGException(f"Could not get leafs: {err}") diff --git a/keepercommander/keeper_dag/connection/ksm.py b/keepercommander/keeper_dag/connection/ksm.py index 58fa4fac0..e24b85592 100644 --- a/keepercommander/keeper_dag/connection/ksm.py +++ b/keepercommander/keeper_dag/connection/ksm.py @@ -165,6 +165,7 @@ def authenticate(self, attempt = 0 while True: + err_msg = "no error message" try: attempt += 1 response = requests.get(url, diff --git a/keepercommander/keeper_dag/connection/local.py b/keepercommander/keeper_dag/connection/local.py index 6a0dac42b..0567860d5 100644 --- a/keepercommander/keeper_dag/connection/local.py +++ b/keepercommander/keeper_dag/connection/local.py @@ -582,6 +582,41 @@ def sync(self, hasMore=has_more ).model_dump_json().encode() + def multi_sync(self, + multi_query: Union[gs_pb2.GraphSyncMultiQuery, Any], + graph_id: Optional[int] = None, + endpoint: Optional[str] = None, + agent: Optional[str] = None) -> bytes: + """Local mirror of the network per-graph ``multi_sync``. + + The local SQLite store has no per-graph URL routing, so each sub-query + in the ``GraphSyncMultiQuery`` is run through this connection's own + ``sync()`` — identical stream / sync-point / graph-id semantics as the + single-stream read and the save path — and the per-stream results are + assembled into the same multi-stream envelope the network endpoint + returns: ``GraphSyncMultiResult`` (protobuf) or ``{"results": [...]}`` + (JSON). + """ + is_protobuf = isinstance(multi_query, gs_pb2.GraphSyncMultiQuery) + queries = list(multi_query.queries) + + if is_protobuf: + multi = gs_pb2.GraphSyncMultiResult() + for sub_query in queries: + single = self.sync(sub_query, graph_id=graph_id, + endpoint=endpoint, agent=agent) + result = gs_pb2.GraphSyncResult() + result.ParseFromString(single) + multi.results.add().CopyFrom(result) + return multi.SerializeToString() + + results = [] + for sub_query in queries: + single = self.sync(sub_query, graph_id=graph_id, + endpoint=endpoint, agent=agent) + results.append(json.loads(single)) + return json.dumps({"results": results}).encode() + def debug_dump(self) -> str: ret = "" diff --git a/keepercommander/keeper_dag/dag.py b/keepercommander/keeper_dag/dag.py index 85a2d1296..a9066528e 100644 --- a/keepercommander/keeper_dag/dag.py +++ b/keepercommander/keeper_dag/dag.py @@ -15,7 +15,7 @@ import importlib import traceback import sys -from typing import Optional, Union, List, Any, Tuple, TYPE_CHECKING +from typing import Optional, Union, List, Any, Tuple, Dict, TYPE_CHECKING if TYPE_CHECKING: from .connection import ConnectionBase @@ -225,7 +225,7 @@ def close(self): try: # Safely get the root vertex without creating a new one if hasattr(self, '_vertices') and hasattr(self, 'uid') and hasattr(self, '_uid_lookup'): - if len(self._vertices) > 0 and self.uid in self._uid_lookup: + if len(self._vertices) > 0 and self.uid is not None and self.uid in self._uid_lookup: idx = self._uid_lookup[self.uid] if idx < len(self._vertices): root = self._vertices[idx] @@ -298,7 +298,7 @@ def debug_stacktrace(self): trc = 'Traceback (most recent call last):\n' msg = trc + ''.join(traceback.format_list(stack)) if exc is not None: - msg += ' ' + traceback.format_exc().lstrip(trc) + msg += ' ' + traceback.format_exc().removeprefix(trc) self.debug(msg) def __str__(self): @@ -310,6 +310,8 @@ def __str__(self): for v in self.all_vertices: ret += f" * {v.uid}, Keys: {v.keychain}, Active: {v.active}\n" for e in v.edges: + if e is None: + continue if e.edge_type == EdgeType.DATA: ret += " + has a DATA edge" if e.content is not None: @@ -504,11 +506,28 @@ def get_vertices_by_path_value(self, path: str, inc_deleted: bool = False) -> Li for vertex in vertices: for edge in vertex.edges: - if edge.path == path: + if edge is not None and edge.path == path: results.append(vertex) return results def _sync(self, sync_point: int = 0) -> Tuple[List[DAGData], int]: + """Dispatch to legacy single-stream sync or per-graph multi-stream sync. + + When `read_endpoint` is set, the server uses the per-graph URL pattern + (`/api/user/graph-sync//...`). That model splits the graph across + multiple streams, so a single-stream `sync` returns only a fragment. + Web Vault uses `get_leafs` -> `multi_sync` to read the full graph; + this client follows the same pattern. + + When only `graph_id` is set (legacy single-endpoint transport), the + single-stream sync remains correct. + """ + if self.read_endpoint is not None: + return self._sync_per_graph(sync_point) + return self._sync_legacy(sync_point) + + def _sync_legacy(self, sync_point: int = 0) -> Tuple[List[DAGData], int]: + """Single-stream sync against the legacy `/sync` endpoint.""" # The web service will send 500 items, if there is more the 'has_more' flag is set to True. has_more = True @@ -543,6 +562,61 @@ def _sync(self, sync_point: int = 0) -> Tuple[List[DAGData], int]: return all_data, sync_point + def _sync_per_graph(self, sync_point: int = 0) -> Tuple[List[DAGData], int]: + """Multi-stream read against the per-graph endpoints. + + The graph's data lives in a single stream keyed by the graph's origin + (e.g. the PAM Configuration record's UID for TunnelDAG). We multi_sync + that stream directly — no `get_leafs` discovery step needed for this + caller pattern. (`Connection.get_leafs` remains available for callers + that start from leaf vertices and need to discover stream roots.) + + Returns aggregated (data, max_sync_point) just like `_sync_legacy`. + """ + + origin_bytes = urlsafe_str_to_bytes(self.uid) + + # Stream keyed by the graph's origin (e.g. config_uid for PAM linking). + per_stream_sync_point: Dict[bytes, int] = {origin_bytes: sync_point} + all_data: List[DAGData] = [] + max_sync_point = sync_point + + while per_stream_sync_point: + stream_ids = list(per_stream_sync_point.keys()) + multi_query = self.read_struct_obj.multi_sync_query( + stream_ids=stream_ids, + origin=origin_bytes, + sync_point=sync_point, + ) + # Per-stream syncPoint adjustment so each stream advances + # independently across pagination rounds (proto variant only; + # JSON variant builds via SyncQuery which already carries syncPoint). + try: + for inner, sid in zip(multi_query.queries, stream_ids): + inner.syncPoint = per_stream_sync_point[sid] + except Exception: # pragma: no cover - JSON variant has no .queries + pass + + multi_response = self.conn.multi_sync( + multi_query=multi_query, + graph_id=self.graph_id, + endpoint=self.read_endpoint, + agent=self.agent, + ) + multi_results = self.read_struct_obj.get_multi_sync_result(multi_response) + + next_per_stream: Dict[bytes, int] = {} + for result in multi_results: + all_data += result.data + if result.syncPoint and result.syncPoint > max_sync_point: + max_sync_point = result.syncPoint + if result.hasMore and result.streamId is not None: + next_per_stream[bytes(result.streamId)] = result.syncPoint + + per_stream_sync_point = next_per_stream + + return all_data, max_sync_point + def _load(self, sync_point: int = 0): """ @@ -619,11 +693,12 @@ def _load(self, sync_point: int = 0): name=data.parentRef.name, vertex_type=RefType.GENERAL ) + # Get the head vertex, which will exist now. - head = self.get_vertex(head_uid) + head = self.get_vertex_by_uid(head_uid) if head is None or head == "": head = tail - head = self.get_vertex_by_uid(head_uid) + self.debug(f" * tail {tail_uid} belongs to {head_uid}, " f"edge type {edge_type}", level=3) diff --git a/keepercommander/keeper_dag/struct/__init__.py b/keepercommander/keeper_dag/struct/__init__.py index 53aa771da..ac87baa7b 100644 --- a/keepercommander/keeper_dag/struct/__init__.py +++ b/keepercommander/keeper_dag/struct/__init__.py @@ -54,3 +54,34 @@ def payload(origin_ref: Union[Ref, gs_pb2.GraphSyncRef], graph_id: Optional[int] = None) -> Union[DataPayload, gs_pb2.GraphSyncAddDataRequest]: pass + + # --- Per-graph multi-stream read transport --------------------------- + # Used by DAG._sync_per_graph when read_endpoint is set. Two-step pattern: + # 1. leafs_query(...) -> get_leafs_result(...) discovers stream refs. + # 2. multi_sync_query(...) -> get_multi_sync_result(...) fetches data. + + def leafs_query(self, + vertices: List[str]) -> Union[BaseModel, gs_pb2.GraphSyncLeafsQuery]: + """Build a GraphSyncLeafsQuery from a list of vertex UIDs (URL-safe str).""" + pass + + @staticmethod + def get_leafs_result(results: bytes) -> List[Ref]: + """Parse GraphSyncRefsResult bytes into a list of Ref objects. + Each Ref's `value` is the stream UID rooted under the queried vertex. + """ + pass + + def multi_sync_query(self, + stream_ids: List[bytes], + origin: bytes, + sync_point: int = 0) -> Union[BaseModel, gs_pb2.GraphSyncMultiQuery]: + """Build a GraphSyncMultiQuery wrapping one GraphSyncQuery per stream.""" + pass + + @staticmethod + def get_multi_sync_result(results: bytes): # -> List[SyncData] + """Parse GraphSyncMultiResult bytes into a list of SyncData, one per + inner GraphSyncResult (each carrying its own streamId/syncPoint/hasMore). + """ + pass diff --git a/keepercommander/keeper_dag/struct/default.py b/keepercommander/keeper_dag/struct/default.py index a58558bff..dd23a6256 100644 --- a/keepercommander/keeper_dag/struct/default.py +++ b/keepercommander/keeper_dag/struct/default.py @@ -1,8 +1,10 @@ from __future__ import annotations +import json from . import DataStructBase from ..types import SyncQuery, Ref, RefType, DAGData, DataPayload, EdgeType, SyncData -from ..crypto import generate_random_bytes, generate_uid_str, bytes_to_str +from ..crypto import generate_random_bytes, generate_uid_str, bytes_to_str, bytes_to_urlsafe_str import base64 +from pydantic import BaseModel from typing import Optional, List @@ -79,3 +81,62 @@ def payload(origin_ref: Ref, dataList=data_list, graphId=graph_id ) + + # --- Per-graph multi-stream read transport --------------------------- + + class _LeafsQuery(BaseModel): + vertices: List[str] + + class _MultiSyncQuery(BaseModel): + queries: List[SyncQuery] + + def leafs_query(self, vertices: List[str]) -> 'DataStruct._LeafsQuery': + return DataStruct._LeafsQuery(vertices=list(vertices)) + + @staticmethod + def get_leafs_result(results: bytes) -> List[Ref]: + try: + obj = json.loads(results) + except Exception as err: + raise Exception(f"Could not parse the leafs JSON result: {err}") + refs_list = obj.get("refs", []) if isinstance(obj, dict) else obj + out: List[Ref] = [] + for r in refs_list: + # Server may return either {type, value, name} or just a value str. + if isinstance(r, dict): + value = r.get("value") + if isinstance(value, bytes): + value = bytes_to_urlsafe_str(value) + out.append(Ref( + type=RefType(r["type"]) if r.get("type") is not None else RefType.GENERAL, + value=value, + name=r.get("name") or None, + )) + return out + + def multi_sync_query(self, + stream_ids: List[bytes], + origin: bytes, + sync_point: int = 0) -> 'DataStruct._MultiSyncQuery': + queries = [ + SyncQuery( + streamId=bytes_to_urlsafe_str(sid), + deviceId=bytes_to_urlsafe_str(origin), + syncPoint=sync_point, + graphId=None, + ) + for sid in stream_ids + ] + return DataStruct._MultiSyncQuery(queries=queries) + + @staticmethod + def get_multi_sync_result(results: bytes) -> List[SyncData]: + try: + obj = json.loads(results) + except Exception as err: + raise Exception(f"Could not parse the multi_sync JSON result: {err}") + items = obj.get("results", []) if isinstance(obj, dict) else obj + out: List[SyncData] = [] + for item in items: + out.append(SyncData.model_validate(item)) + return out diff --git a/keepercommander/keeper_dag/struct/protobuf.py b/keepercommander/keeper_dag/struct/protobuf.py index fcd123d5d..7a449f918 100644 --- a/keepercommander/keeper_dag/struct/protobuf.py +++ b/keepercommander/keeper_dag/struct/protobuf.py @@ -58,23 +58,16 @@ def sync_query(self, ) @staticmethod - def get_sync_result(results: bytes) -> SyncData: - - try: - result = gs_pb2.GraphSyncResult() - result.ParseFromString(results) - except Exception as err: - raise Exception(f"Could not parse the GraphSyncResult message: {err}") - - message = gs_pb2.GraphSyncResult() - message.ParseFromString(results) - + def _sync_data_from_result(message: gs_pb2.GraphSyncResult) -> SyncData: + """Convert a single GraphSyncResult protobuf into a SyncData pydantic + model. Extracted so both single-`sync` and multi_sync code paths share + identical per-result decoding. + """ data_list: List[SyncDataItem] = [] for item in message.data: data_list.append( SyncDataItem( type=DataStruct.PB_TO_DATA_MAP.get(item.data.type), - # content=bytes_to_str(item.data.content), content=item.data.content, content_is_base64=False, ref=Ref( @@ -92,9 +85,21 @@ def get_sync_result(results: bytes) -> SyncData: return SyncData( syncPoint=message.syncPoint, data=data_list, - hasMore=message.hasMore + hasMore=message.hasMore, + streamId=bytes(message.streamId) if message.streamId else None, ) + @staticmethod + def get_sync_result(results: bytes) -> SyncData: + + try: + message = gs_pb2.GraphSyncResult() + message.ParseFromString(results) + except Exception as err: + raise Exception(f"Could not parse the GraphSyncResult message: {err}") + + return DataStruct._sync_data_from_result(message) + @staticmethod def origin_ref(origin_ref_value: bytes, name: str) -> gs_pb2.GraphSyncRef: @@ -149,3 +154,49 @@ def payload(origin_ref: gs_pb2.GraphSyncRef, return gs_pb2.GraphSyncAddDataRequest( origin=origin_ref, data=data_list) + + # --- Per-graph multi-stream read transport --------------------------- + + def leafs_query(self, vertices: List[str]) -> gs_pb2.GraphSyncLeafsQuery: + return gs_pb2.GraphSyncLeafsQuery( + vertices=[urlsafe_str_to_bytes(v) for v in vertices] + ) + + @staticmethod + def get_leafs_result(results: bytes) -> List[Ref]: + msg = gs_pb2.GraphSyncRefsResult() + try: + msg.ParseFromString(results) + except Exception as err: + raise Exception(f"Could not parse the GraphSyncRefsResult message: {err}") + return [ + Ref( + type=DataStruct.PB_TO_REF_MAP.get(r.type), + value=bytes_to_urlsafe_str(r.value), + name=r.name or None, + ) + for r in msg.refs + ] + + def multi_sync_query(self, + stream_ids: List[bytes], + origin: bytes, + sync_point: int = 0) -> gs_pb2.GraphSyncMultiQuery: + return gs_pb2.GraphSyncMultiQuery(queries=[ + gs_pb2.GraphSyncQuery( + streamId=sid, + origin=origin, + syncPoint=sync_point, + maxCount=0, # let krouter default (currently 500) + ) + for sid in stream_ids + ]) + + @staticmethod + def get_multi_sync_result(results: bytes) -> List[SyncData]: + msg = gs_pb2.GraphSyncMultiResult() + try: + msg.ParseFromString(results) + except Exception as err: + raise Exception(f"Could not parse the GraphSyncMultiResult message: {err}") + return [DataStruct._sync_data_from_result(r) for r in msg.results] diff --git a/keepercommander/keeper_dag/types.py b/keepercommander/keeper_dag/types.py index 9ab242c88..4b614fbfe 100644 --- a/keepercommander/keeper_dag/types.py +++ b/keepercommander/keeper_dag/types.py @@ -124,6 +124,17 @@ class PamEndpoints(BaseEnum): PamGraphId.SERVICE_LINKS.value: PamEndpoints.SERVICE_LINKS, } +# Inverse map for callers that have a graph_id int and need the PamEndpoints enum +# to address the new /api/user/graph-sync// routes. +GRAPH_ID_TO_ENDPOINT = { + PamGraphId.PAM.value: PamEndpoints.PAM, + PamGraphId.DISCOVERY_RULES.value: PamEndpoints.DISCOVERY_RULES, + PamGraphId.DISCOVERY_JOBS.value: PamEndpoints.DISCOVERY_JOBS, + PamGraphId.INFRASTRUCTURE.value: PamEndpoints.INFRASTRUCTURE, + PamGraphId.SERVICE_LINKS.value: PamEndpoints.SERVICE_LINKS, +} + + class SyncQuery(BaseModel): streamId: Optional[str] = None # base64 of a user's ID who is syncing. deviceId: Optional[str] = None @@ -134,7 +145,10 @@ class SyncQuery(BaseModel): class SyncDataItem(BaseModel): ref: Ref parentRef: Optional[Ref] = None - content: Optional[str] = None + # Either a base64-encoded string (JSON wire format) or raw bytes + # (protobuf wire format). `content_is_base64` distinguishes them so the + # consumer can decode appropriately. + content: Optional[Union[str, bytes]] = None content_is_base64: bool = True type: Optional[str] = None path: Optional[str] = None @@ -145,6 +159,9 @@ class SyncData(BaseModel): syncPoint: int data: List[SyncDataItem] hasMore: bool + # Per-graph multi_sync: identifies which stream this result came from. + # None for single-stream `sync` results (backward compatible). + streamId: Optional[bytes] = None class Ref(BaseModel): diff --git a/keepercommander/keeper_dag/utils.py b/keepercommander/keeper_dag/utils.py index 43ad5a76e..51e33c921 100644 --- a/keepercommander/keeper_dag/utils.py +++ b/keepercommander/keeper_dag/utils.py @@ -55,5 +55,5 @@ def set_file_permissions(file_path): # type: (str) -> None check=False, capture_output=True) subprocess.run(["icacls", file_path, "/grant", f"{username}:M"], check=True, capture_output=True) logging.debug(f'Set secure permissions (owner Modify only) for Windows file: {file_path}') - except Exception: - logging.warning(f'Failed to set file permissions for {file_path}') + except (OSError, subprocess.SubprocessError) as err: + logging.warning(f'Failed to set file permissions for {file_path}: {err}') From 14c0df7db1ec853a60211c693447ffbb3d59ab56 Mon Sep 17 00:00:00 2001 From: John Walstra Date: Mon, 8 Jun 2026 11:47:30 -0500 Subject: [PATCH 5/8] chore: Allow a CLAUDE.md to help guide AI. --- .gitignore | 2 +- CLAUDE.md | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 CLAUDE.md diff --git a/.gitignore b/.gitignore index 1b40c8dc0..e81f363fa 100644 --- a/.gitignore +++ b/.gitignore @@ -17,7 +17,7 @@ Makefile dr-logs /.venv* *.swp -CLAUDE.md +#CLAUDE.md AGENTS.md keeper_db.sqlite __pycache__/ diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..f8b777f2e --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,97 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## ⚠️ Do not edit vendored directories + +**Never edit files under `keepercommander/discovery_common/` or `keepercommander/keeper_dag/`.** These are copied in from external "golden" repositories (`keeper-dag`, and the shared Gateway/KDNRM `discovery-common` code). Any edits made here will be **overwritten** the next time the directories are synced from upstream. If a change is needed in this code, make it in the upstream repo — not here. The same applies to generated protobuf files in `keepercommander/proto/` (`*_pb2.py` / `*.pyi`): regenerate from the `.proto` source, never hand-edit. + +## What this is + +Keeper Commander is a command-line and terminal-UI client for Keeper Password Manager and KeeperPAM. It is a Python package (`keepercommander`) that ships as the `keeper` console script. Beyond vault access it does enterprise administration, PAM (privileged access: rotation, tunnels, discovery), data import/export from other password managers, and can run as a REST service. + +## Setup & running + +```bash +python3 -m venv venv +source venv/bin/activate +pip install -r requirements.txt +pip install -e . +pip install -e '.[email]' # optional email-sending extras +``` + +Run the CLI: +- `keeper help` — list all commands +- `keeper shell` — interactive command shell +- `keeper supershell` — full terminal vault UI (Textual) +- `keeper [args]` — run a single command and exit +- `python keeper.py ...` — equivalent entry point (calls `keepercommander.__main__:main`) + +Config defaults to `config.json` (cwd or platform data dir); override with `KEEPER_CONFIG_FILE`. Set `KEEPER_COMMANDER_DEBUG` for debug logging. + +## Testing + +CI runs only `unit-tests/` on Python 3.8 and 3.14 (`.github/workflows/test-with-pytest.yml`). Supported range is 3.8–3.14. + +```bash +pip install '.[test]' +pytest unit-tests/ # what CI runs +pytest unit-tests/test_sync_down.py # single file +pytest unit-tests/test_sync_down.py::TestClass::test_name # single test +pytest -m keeper_imports # smoke test: every module imports cleanly +``` + +Test marker semantics (see `pytest.ini`): +- `unit` — mocked, no live server (credential-provision) +- `e2e` — end-to-end, run manually (`pytest -m e2e`) +- `integration` — hits internal `dev.keepersecurity.com` accounts via a `config.json` (`tests/data_config.py`); not for general use +- `cross_enterprise` — excluded by default in `addopts` + +Note: `pytest.ini` excludes `venv/`, ignores `unit-tests/test_command_utils.py` (circular import) and `unit-tests/test_login.py` (connection errors), and disables warnings. The `tests/` directory holds heavier integration/e2e suites that are *not* run by CI. + +Lint config is `pylintrc`; `desired-pylint-warnings` documents which warning categories the team cares about. + +## Architecture + +**Entry & dispatch.** `__main__.py` loads config into a `KeeperParams`, then `cli.py` drives the REPL/one-shot execution. `cli.do_command()` is the central dispatcher: it parses a command line, resolves aliases, picks the right registry (`commands`, `enterprise_commands`, or `msp_commands`), and calls the command. Control characters in command input are rejected at this boundary. + +**Commands.** Every command subclasses `Command` (or `ArgparseCommand`) in `keepercommander/commands/base.py`. The contract: +- `get_parser()` returns an `argparse.ArgumentParser`; `execute_args()` parses the raw string and dispatches to `execute(params, **kwargs)`. +- `is_authorised()` gates whether login is required. +- `GroupCommand` / `GroupCommandNew` compose sub-verbs (e.g. `pam `); they own their own sub-registries and aliases. +- Mixins `RecordMixin` / `FolderMixin` provide shared record/folder resolution helpers. + +Commands are registered through `register_commands(commands, aliases, command_info)` in `commands/base.py`, which imports each command module and calls its `register_commands` / `register_command_info`. To add a command, create the module, implement the `Command`, and wire it into the appropriate `register_commands` function. The `commands/` subdirectories group large feature areas (`pam`, `pam_cloud`, `pam_import`, `discover`, `enterprise*`, `domain_management`, `remote_management`, `keeper_drive`, `pedm`, `scim`). + +**Session & state.** `KeeperParams` (`params.py`) is the single mutable object threaded through everything: session tokens, the in-memory vault cache (records, folders, shared folders, teams), enterprise data, and `RestApiContext` (server, transmission/encryption keys, QRC/EC key negotiation). Commands read and mutate this object rather than passing data around. + +**Network & data sync.** `api.py` is the transport layer: `login()`, `communicate()` / `communicate_rest()` (protobuf request/response with throttle retry), and `query_enterprise()`. `rest_api.py` / `loginv3.py` handle the low-level REST and login-v3 flows; `auth/` holds login-step and console-UI logic. `sync_down.py` pulls and decrypts the vault into `params`, then `prepare_folder_tree()` builds the folder hierarchy. Wire formats live in `proto/` (generated `*_pb2.py` — do not hand-edit). Crypto primitives are in `crypto.py`. + +**Vault data model.** `vault.py` defines the record types: `KeeperRecord` (abstract) with `PasswordRecord` (v2), `TypedRecord` (v3, field-based with `TypedField`), `FileRecord`, `ApplicationRecord`. `record_facades.py` / `vault_extensions.py` provide typed views; `subfolder.py` models the folder tree. + +**Local storage.** `storage/` (SQLite + in-memory DAOs) and `config_storage/` persist cache and config; secure config storage can be encrypted (`loader.SecureStorageException` path in `__main__.py`). + +**PAM / discovery / graph.** `keeper_dag/` and `discovery_common/` implement the directed-acyclic graph (DAG) backing PAM discovery, record-linking, and rotation. `commands/pam/`, `commands/pam_cloud/`, and `commands/discover/` build on top of them. These two directories are **vendored copies** of external golden repos — see the warning at the top of this file before touching them. + +**Importers.** `importer/` has per-product subpackages (1password, bitwarden, lastpass, keepass, dashlane, proton, thycotic, cyberark, etc.) plus generic csv/json. `imp_exp.py` orchestrates import/export. + +**Service mode.** `service/` is a Flask-based REST API server exposing Commander commands over HTTP with API-key auth, rate limiting, and optional response encryption. See `keepercommander/service/README.md`. Managed via `service-create` / `service-start` / etc. commands. + +**Plugins.** `plugins/` are rotation plugins loaded dynamically and registered like other commands. + +## Style + +Follow [PEP 8](https://peps.python.org/pep-0008/), with the project-specific settings enforced by `pylintrc`: +- **Line length: 100** (not PEP 8's default 79). +- `snake_case` for functions, methods, arguments, variables, and attributes. +- `PascalCase` for classes. +- `UPPER_CASE` for module-level and class constants. +- 4-space indentation, no tabs. + +Run `pylint keepercommander/.py` to check; `desired-pylint-warnings` documents which warning categories the team treats as meaningful. + +## Conventions + +- Match the surrounding file's style; most modules carry the Keeper ASCII-art header. +- Never edit `keeper_dag/`, `discovery_common/`, or generated `proto/` files (see the warning at the top of this file). +- Version lives in `keepercommander/__init__.py` (`__version__`); `setup.cfg` reads it via `attr:`. \ No newline at end of file From e212477d60aad22bc8396897d5afd544a2fc97e9 Mon Sep 17 00:00:00 2001 From: Ayrris Aunario <105313137+aaunario-keeper@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:50:52 -0500 Subject: [PATCH 6/8] feat: desktop bridge login via Keeper Desktop (KC-1300) - Add --via-desktop flag to top-level CLI and login command - Auto-suppress startup login when first queued command is login --via-desktop - Implement exchange_vault_token bridge relay + direct KA login_from_existing_session_token - Auto-enroll device on fresh install (no config.json required) - Register DEDK after bridge login for future persistent logins - 12 unit tests covering bridge exchange, KA transport, auto-enrollment, error paths --- keepercommander/__main__.py | 13 +- keepercommander/auth/desktop_bridge.py | 273 ++++++++++++++++ keepercommander/cli.py | 20 +- keepercommander/commands/utils.py | 73 +++-- keepercommander/params.py | 2 + unit-tests/test_desktop_bridge_login.py | 394 ++++++++++++++++++++++++ 6 files changed, 746 insertions(+), 29 deletions(-) create mode 100644 keepercommander/auth/desktop_bridge.py create mode 100644 unit-tests/test_desktop_bridge_login.py diff --git a/keepercommander/__main__.py b/keepercommander/__main__.py index b627a7947..7b9dbe35a 100644 --- a/keepercommander/__main__.py +++ b/keepercommander/__main__.py @@ -120,6 +120,7 @@ def show_brief_help(): print(' --batch-mode Run in batch/non-interactive mode') print(' --proxy PROXY Proxy server') print(' --new-login Force full login (bypass persistent login)') + print(' --via-desktop Use Keeper Desktop bridge for login') print(' --version Display version') print('') print('Getting Started:') @@ -163,6 +164,8 @@ def show_brief_help(): parser.add_argument('--fail-on-throttle', action='store_true', help=fail_on_throttle_help) parser.add_argument('--data-dir', dest='data_dir', action='store', help='Directory to use for Commander data (config, cache, etc.). Overrides environment variables.') parser.add_argument('--new-login', dest='new_login', action='store_true', help='Force full login flow (bypass persistent login)') +parser.add_argument('--via-desktop', dest='via_desktop', action='store_true', + help='Use Keeper Desktop bridge for login in this Commander process') parser.add_argument('--config-file', dest='config_file', action='store_true', help='Store credentials in config.json instead of the OS-native keychain') parser.add_argument('command', nargs='?', type=str, action='store', help='Command') @@ -203,6 +206,9 @@ def main(from_package=False): sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0]) opts, flags = parser.parse_known_args(sys.argv[1:]) + if opts.via_desktop and opts.new_login: + logging.error('--via-desktop cannot be used with --new-login') + sys.exit(1) # Store the original command arguments for proper reconstruction if opts.command: @@ -226,8 +232,8 @@ def main(from_package=False): '--launched-with-shortcut', '--proxy', '--data-dir', '-ks', '-ku', '-kp', '-lwsc'] bool_main_parser_args = ['--version', '--debug', '--silent', '--batch-mode', - '--unmask-all', '--fail-on-throttle', - '--new-login', '--config-file'] + '--unmask-all', '--fail-on-throttle', + '--new-login', '--via-desktop', '--config-file'] main_parser_args = value_main_parser_args + bool_main_parser_args is_main_parser_arg = False @@ -298,6 +304,9 @@ def main(from_package=False): if opts.fail_on_throttle: params.rest_context.fail_on_throttle = opts.fail_on_throttle + params.via_desktop_login = opts.via_desktop + params.top_level_new_login = opts.new_login + if opts.password: params.password = opts.password else: diff --git a/keepercommander/auth/desktop_bridge.py b/keepercommander/auth/desktop_bridge.py new file mode 100644 index 000000000..c21e3906a --- /dev/null +++ b/keepercommander/auth/desktop_bridge.py @@ -0,0 +1,273 @@ +# -*- coding: utf-8 -*- +# _ __ +# | |/ /___ ___ _ __ ___ _ _ ® +# | ' None + module = bridge_module or _load_bridge_module() + device_token, device_private_key = _get_device_credentials(params) + request = _build_bootstrap_request(module, params, device_token, device_private_key, bridge_socket, timeout_ms) + + try: + vault_result = module.BridgeClient().exchange_vault_token(request) + except Exception as exc: + raise _translate_bridge_error(exc) from exc + + vault_session_token = bytes(vault_result.vault_session_token) + uid = request.message_session_uid + if isinstance(uid, str): + message_session_uid = utils.base64_url_decode(uid) + elif uid: + message_session_uid = bytes(uid) + else: + message_session_uid = b'' + + encrypted_session_token, encrypted_data_key, primary_username, account_uid = \ + _ka_login_from_existing_session_token( + params, device_token, message_session_uid, vault_session_token, + ) + + _apply_ka_credentials( + params, device_private_key, encrypted_session_token, encrypted_data_key, + primary_username, account_uid, + ) + logging.info('Authenticated through Keeper Desktop bridge.') + + +def _ka_login_from_existing_session_token(params, encrypted_device_token, message_session_uid, vault_session_token): + # type: (KeeperParams, bytes, bytes, bytes) -> tuple + """ + Calls KA authentication/login_from_existing_session_token (protocol contract step 6). + + Header context: vault session token in ApiRequestPayload.encryptedSessionToken. + Body: StartLoginRequest with encryptedDeviceToken, clientVersion, messageSessionUid. + + Returns (encrypted_session_token, encrypted_data_key, primary_username, account_uid). + Raises DesktopBridgeLoginError on any failure. + """ + from ..proto import APIRequest_pb2 + from .. import rest_api as _rest_api + + rq = APIRequest_pb2.StartLoginRequest() + rq.clientVersion = _rest_api.CLIENT_VERSION + rq.encryptedDeviceToken = bytes(encrypted_device_token) + rq.messageSessionUid = bytes(message_session_uid) + + payload = APIRequest_pb2.ApiRequestPayload() + payload.payload = rq.SerializeToString() + payload.encryptedSessionToken = bytes(vault_session_token) + + try: + rs = _rest_api.execute_rest( + params.rest_context, + 'authentication/login_from_existing_session_token', + payload, + ) + except Exception as exc: + raise DesktopBridgeLoginError( + f'KA login_from_existing_session_token request failed: {exc}', + code=KDBC_KA_LOGIN_FAILED, + kind='ka_login_failed', + actor='ka', + ) from exc + + if isinstance(rs, dict): + raise DesktopBridgeLoginError( + f"KA login_from_existing_session_token error: {rs.get('message') or rs.get('error') or 'unknown'}", + code=KDBC_KA_LOGIN_FAILED, + kind='ka_login_failed', + actor='ka', + ) + + if not isinstance(rs, bytes): + raise DesktopBridgeLoginError( + 'KA login_from_existing_session_token returned unexpected response type', + code=KDBC_KA_LOGIN_FAILED, + kind='ka_login_failed', + actor='ka', + ) + + login_resp = APIRequest_pb2.LoginResponse() + login_resp.ParseFromString(rs) + + if login_resp.loginState != APIRequest_pb2.LoginState.Value('LOGGED_IN'): + raise DesktopBridgeLoginError( + f'KA login_from_existing_session_token returned unexpected login state: {login_resp.loginState}', + code=KDBC_KA_LOGIN_FAILED, + kind='ka_login_failed', + actor='ka', + ) + + if not login_resp.encryptedSessionToken or not login_resp.encryptedDataKey: + raise DesktopBridgeLoginError( + 'KA login_from_existing_session_token response missing required credential fields', + code=KDBC_KA_LOGIN_FAILED, + kind='ka_login_failed', + actor='ka', + ) + + return ( + login_resp.encryptedSessionToken, + login_resp.encryptedDataKey, + login_resp.primaryUsername or None, + login_resp.accountUid or None, + ) + + +def _load_bridge_module(): + try: + return importlib.import_module('keeper_desktop_bridge_client') + except ImportError as exc: + raise DesktopBridgeLoginError( + 'Python package keeper_desktop_bridge_client is not available.', + code=KDBC_INTERNAL_ERROR, + kind='dependency_missing', + actor='commander' + ) from exc + + +def _get_device_credentials(params): + if not params.device_token or not params.device_private_key: + logging.info('No device credentials found — registering new device with KA.') + try: + from .. import loginv3 + loginv3.LoginV3API.get_device_id(params) + except Exception as exc: + raise DesktopBridgeLoginError( + f'Device registration failed: {exc}', + code=KDBC_CLIENT_NOT_ENROLLED, + kind='not_enrolled', + actor='commander', + ) from exc + + try: + device_token = utils.base64_url_decode(params.device_token) + device_private_key = utils.base64_url_decode(params.device_private_key) + crypto.load_ec_private_key(device_private_key) + except Exception as exc: + raise DesktopBridgeLoginError( + 'Stored Commander device credentials are invalid.', + code=KDBC_CLIENT_NOT_ENROLLED, + kind='not_enrolled', + actor='commander' + ) from exc + + return device_token, device_private_key + + +def _build_bootstrap_request(module, params, device_token, device_private_key, bridge_socket, timeout_ms): + private_key = crypto.load_ec_private_key(device_private_key) + device_public_key = crypto.unload_ec_public_key(private_key.public_key()) + client = module.ClientIdentity( + name='Keeper Commander', + version=__version__, + kind='commander', + ka_client_version=rest_api.CLIENT_VERSION, + ) + device = module.DeviceCredentials( + encrypted_device_token=device_token, + device_private_key=device_private_key, + device_public_key=device_public_key, + ) + + config = None + if hasattr(module, 'BridgeClientConfig'): + config = module.BridgeClientConfig( + server=params.server, + socket_override=bridge_socket, + timeout_millis=timeout_ms, + ) + + return module.BootstrapRequest( + client=client, + device=device, + flow='already_enrolled', + request_id=str(uuid.uuid4()), + message_session_uid=utils.generate_uid(), + config=config, + ) + + +def _translate_bridge_error(exc): + code = getattr(exc, 'code', None) or KDBC_INTERNAL_ERROR + kind = getattr(exc, 'kind', None) or 'bridge_error' + retryable = bool(getattr(exc, 'retryable', False)) + actor = getattr(exc, 'actor', None) or 'bridge' + request_id = getattr(exc, 'request_id', None) + message = getattr(exc, 'message', None) or str(exc) or 'Unknown bridge error' + return DesktopBridgeLoginError( + message, + code=code, + kind=kind, + retryable=retryable, + actor=actor, + request_id=request_id, + ) + + +def _apply_ka_credentials(params, device_private_key, encrypted_session_token, encrypted_data_key, + primary_username, account_uid): + params.session_token_bytes = bytes(encrypted_session_token) + params.session_token = utils.base64_url_encode(params.session_token_bytes) + private_key = crypto.load_ec_private_key(device_private_key) + params.data_key = crypto.decrypt_ec(bytes(encrypted_data_key), private_key) + params.password = None + params.clone_code = None + + if primary_username: + params.user = primary_username + + if account_uid: + if isinstance(account_uid, bytes): + params.account_uid_bytes = account_uid + else: + try: + params.account_uid_bytes = utils.base64_url_decode(account_uid) + except Exception: + logging.debug('KA login returned an account UID with an unexpected encoding.') + + from .. import loginv3 + loginv3.LoginV3Flow.populateAccountSummary(params) diff --git a/keepercommander/cli.py b/keepercommander/cli.py index 09abef0a9..ff23c56b4 100644 --- a/keepercommander/cli.py +++ b/keepercommander/cli.py @@ -744,6 +744,24 @@ def read_command_with_continuation(prompt_session, params): return result +def _first_queued_command_is_login(params): + if not params.commands: + return False + command = params.commands[0].strip() + if command.startswith('@'): + command = command[1:].strip() + cmd, _ = command_and_args_from_cmd(command) + if not cmd: + return False + cmd = cmd.lower() + alias = aliases.get(cmd) + if isinstance(alias, (tuple, list)): + cmd = alias[0] + elif isinstance(alias, str): + cmd = alias + return cmd == 'login' + + def loop(params, skip_init=False, suppress_goodbye=False, new_login=False): # type: (KeeperParams, bool, bool, bool) -> int # suppress_goodbye kept for API compat global prompt_session error_no = 0 @@ -776,7 +794,7 @@ def _(event): display.show_government_warning() if not params.batch_mode and not skip_init: - if params.user: + if params.user and not _first_queued_command_is_login(params): try: LoginCommand().execute(params, email=params.user, password=params.password, new_login=new_login) except KeyboardInterrupt: diff --git a/keepercommander/commands/utils.py b/keepercommander/commands/utils.py index 9980f365c..6c9ecbbc6 100644 --- a/keepercommander/commands/utils.py +++ b/keepercommander/commands/utils.py @@ -294,7 +294,10 @@ def register_command_info(aliases, command_info): login_parser = argparse.ArgumentParser(prog='login', description='Start a login session on Commander') login_parser.add_argument('-p', '--pass', dest='password', action='store', help='master password') -login_parser.add_argument('--new-login', dest='new_login', action='store_true', help='Force full login flow') +login_mode_group = login_parser.add_mutually_exclusive_group() +login_mode_group.add_argument('--new-login', dest='new_login', action='store_true', help='Force full login flow') +login_mode_group.add_argument('--via-desktop', dest='via_desktop', action='store_true', + help='Use Keeper Desktop bridge for this login') login_parser.add_argument('--server', dest='server', action='store', help='Data center region (US, EU, AU, CA, JP, GOV, etc.)') login_parser.add_argument('--config-file', dest='config_file', action='store_true', help='Store config in config.json instead of the OS-native keychain ' @@ -1690,9 +1693,18 @@ def execute(self, params, **kwargs): user = kwargs.get('email') or '' password = kwargs.get('password') or '' + new_login = kwargs.get('new_login') is True + skip_sync = kwargs.get('skip_sync') is True + via_desktop = kwargs.get('via_desktop') is True or getattr(params, 'via_desktop_login', False) is True + if via_desktop and new_login: + raise CommandError('', '--via-desktop cannot be used with --new-login') + if kwargs.get('via_desktop') is True and getattr(params, 'top_level_new_login', False): + raise CommandError('', '--via-desktop cannot be used with --new-login') + if kwargs.get('via_desktop') is True: + params.via_desktop_login = True try: - if not user: + if not user and not via_desktop: # Show current data center before prompting for email region = get_abbrev_by_host(params.server) if params.server else 'US' if not region: @@ -1703,21 +1715,22 @@ def execute(self, params, **kwargs): print(f'{Fore.CYAN}Use {Fore.GREEN}{hint_cmd}{Fore.CYAN} to change (US, EU, AU, CA, JP, GOV){Fore.RESET}', file=sys.stderr) print('', file=sys.stderr) user = input(f'{Fore.GREEN}Email: {Fore.RESET}').strip() - if not user: + if not user and not via_desktop: return except KeyboardInterrupt as e: logging.info('Canceled') return - params.user = user.lower() + if user: + params.user = user.lower() + elif via_desktop and isinstance(params.config, dict): + params.user = params.config.get('user') or params.user if not password and isinstance(params.config, dict): if 'user' in params.config and 'password' in params.config: if params.config['user'] == params.user: password = params.config['password'] params.password = password - new_login = kwargs.get('new_login') is True - skip_sync = kwargs.get('skip_sync') is True # Apply storage backend choice before login so that store_config_properties # (called inside api.login) honours the user's explicit preference. @@ -1758,17 +1771,25 @@ def execute(self, params, **kwargs): keychain_was_or_would_be_selected = ( not current_backend or _is_os_keychain_url(current_backend) ) - if keychain_was_or_would_be_selected and not is_os_keychain_available(): + if not via_desktop and keychain_was_or_would_be_selected and not is_os_keychain_available(): logging.warning( '%sWarning: OS keychain not available. Config will be stored in config.json ' '(use --config-file to suppress this message).%s', Fore.YELLOW, Fore.RESET ) - try: - api.login(params, new_login=new_login) - except Exception as exc: - logging.warning(str(exc)) + if via_desktop: + try: + from ..auth import desktop_bridge + desktop_bridge.login_via_desktop(params) + except desktop_bridge.DesktopBridgeLoginError as exc: + logging.warning(str(exc)) + return + else: + try: + api.login(params, new_login=new_login) + except Exception as exc: + logging.warning(str(exc)) if params.session_token and not skip_sync: params.enterprise = None @@ -1785,27 +1806,27 @@ def execute(self, params, **kwargs): logging.warning(f'A problem was encountered while updating BreachWatch/security data: {e}') logging.debug(e, exc_info=True) - # Auto-register device for persistent login (stores encrypted data key on server) try: loginv3.LoginV3API.register_encrypted_data_key_for_device(params) except Exception as e: logging.debug(f'Device registration: {e}') - # Print config storage confirmation, mirroring KSM CLI's post-init messages. - stored_backend = (params.config or {}).get(CONFIG_STORAGE_URL, '') - if stored_backend and stored_backend != 'file': - _os = _platform.system() - if _os == 'Darwin': - _storage_label = 'macOS Keychain' - elif _os == 'Windows': - _storage_label = 'Windows Credential Manager' + if not via_desktop: + # Print config storage confirmation, mirroring KSM CLI's post-init messages. + stored_backend = (params.config or {}).get(CONFIG_STORAGE_URL, '') + if stored_backend and stored_backend != 'file': + _os = _platform.system() + if _os == 'Darwin': + _storage_label = 'macOS Keychain' + elif _os == 'Windows': + _storage_label = 'Windows Credential Manager' + else: + _storage_label = 'system keyring (Secret Service)' + logging.info('%sConfig stored in %s.%s', Fore.GREEN, _storage_label, Fore.RESET) else: - _storage_label = 'system keyring (Secret Service)' - logging.info('%sConfig stored in %s.%s', Fore.GREEN, _storage_label, Fore.RESET) - else: - logging.info('%sConfig stored in %s.%s', Fore.GREEN, - os.path.abspath(params.config_filename) if params.config_filename else 'config.json', - Fore.RESET) + logging.info('%sConfig stored in %s.%s', Fore.GREEN, + os.path.abspath(params.config_filename) if params.config_filename else 'config.json', + Fore.RESET) # Show post-login message (only for explicit login command, not auto-login) show_help = kwargs.get('show_help', True) diff --git a/keepercommander/params.py b/keepercommander/params.py index f1c57b328..3a9fbc0c5 100644 --- a/keepercommander/params.py +++ b/keepercommander/params.py @@ -243,6 +243,8 @@ def __init__(self, config_filename='', config=None, server='keepersecurity.com') self.salt = None self.iterations = 0 self.biometric = None + self.via_desktop_login = False + self.top_level_new_login = False self.service_mode = False # Flag to indicate if running in service mode self.thread_local = threading.local() self._pedm_plugin = None # type: diff --git a/unit-tests/test_desktop_bridge_login.py b/unit-tests/test_desktop_bridge_login.py new file mode 100644 index 000000000..41502f5cb --- /dev/null +++ b/unit-tests/test_desktop_bridge_login.py @@ -0,0 +1,394 @@ +import sys +import types +from unittest import TestCase, mock + + +def _install_test_import_stubs(): + zxcvbn_module = types.ModuleType('zxcvbn') + zxcvbn_module.zxcvbn = lambda _: {'score': 0} + sys.modules.setdefault('zxcvbn', zxcvbn_module) + + try: + import google + from google.protobuf import descriptor_pb2, descriptor_pool + except Exception: + return + + pool = descriptor_pool.Default() + + def has_file(name): + try: + pool.FindFileByName(name) + return True + except Exception: + return False + + if not has_file('google/api/http.proto'): + http_proto = descriptor_pb2.FileDescriptorProto() + http_proto.name = 'google/api/http.proto' + http_proto.package = 'google.api' + http_proto.syntax = 'proto3' + http_rule = http_proto.message_type.add() + http_rule.name = 'HttpRule' + + def add_http_rule_field(name, number, field_type): + field = http_rule.field.add() + field.name = name + field.number = number + field.label = descriptor_pb2.FieldDescriptorProto.LABEL_OPTIONAL + field.type = field_type + return field + + add_http_rule_field('selector', 1, descriptor_pb2.FieldDescriptorProto.TYPE_STRING) + add_http_rule_field('get', 2, descriptor_pb2.FieldDescriptorProto.TYPE_STRING) + add_http_rule_field('put', 3, descriptor_pb2.FieldDescriptorProto.TYPE_STRING) + add_http_rule_field('post', 4, descriptor_pb2.FieldDescriptorProto.TYPE_STRING) + add_http_rule_field('delete', 5, descriptor_pb2.FieldDescriptorProto.TYPE_STRING) + add_http_rule_field('patch', 6, descriptor_pb2.FieldDescriptorProto.TYPE_STRING) + add_http_rule_field('body', 7, descriptor_pb2.FieldDescriptorProto.TYPE_STRING) + additional_bindings = add_http_rule_field( + 'additional_bindings', + 11, + descriptor_pb2.FieldDescriptorProto.TYPE_MESSAGE, + ) + additional_bindings.label = descriptor_pb2.FieldDescriptorProto.LABEL_REPEATED + additional_bindings.type_name = '.google.api.HttpRule' + add_http_rule_field('response_body', 12, descriptor_pb2.FieldDescriptorProto.TYPE_STRING) + pool.AddSerializedFile(http_proto.SerializeToString()) + + if not has_file('google/api/annotations.proto'): + annotations_proto = descriptor_pb2.FileDescriptorProto() + annotations_proto.name = 'google/api/annotations.proto' + annotations_proto.package = 'google.api' + annotations_proto.syntax = 'proto3' + annotations_proto.dependency.extend(['google/protobuf/descriptor.proto', 'google/api/http.proto']) + http_extension = annotations_proto.extension.add() + http_extension.name = 'http' + http_extension.number = 72295728 + http_extension.label = descriptor_pb2.FieldDescriptorProto.LABEL_OPTIONAL + http_extension.type = descriptor_pb2.FieldDescriptorProto.TYPE_MESSAGE + http_extension.type_name = '.google.api.HttpRule' + http_extension.extendee = '.google.protobuf.MethodOptions' + pool.AddSerializedFile(annotations_proto.SerializeToString()) + + api_module = sys.modules.setdefault('google.api', types.ModuleType('google.api')) + setattr(google, 'api', api_module) + annotations_module = types.ModuleType('google.api.annotations_pb2') + annotations_module.DESCRIPTOR = pool.FindFileByName('google/api/annotations.proto') + setattr(api_module, 'annotations_pb2', annotations_module) + sys.modules.setdefault('google.api.annotations_pb2', annotations_module) + + +_install_test_import_stubs() + +from keepercommander import cli, crypto, rest_api, utils +from keepercommander.auth import desktop_bridge +from keepercommander.commands import utils as command_utils +from keepercommander.commands.base import ParseError +from keepercommander.error import CommandError +from keepercommander.params import KeeperParams + + +class _VaultBootstrapResult: + def __init__(self, vault_session_token, server=None, expires_in_ms=None, request_id=None): + self.vault_session_token = vault_session_token + self.server = server + self.expires_in_ms = expires_in_ms + self.request_id = request_id or 'test-request-id' + + +class _FakeBridgeClient: + module = None + + def exchange_vault_token(self, request): + self.module.last_request = request + if self.module.error: + raise self.module.error + return self.module.vault_result + + +class _FakeBridgeError(Exception): + def __init__(self): + super().__init__('vault is locked') + self.kind = 'vault_locked' + self.code = 'KDBC_VAULT_LOCKED' + self.retryable = True + self.actor = 'vault' + self.message = 'vault is locked' + self.request_id = 'request-1' + + +def _make_bridge_module(vault_result=None, error=None): + module = types.SimpleNamespace() + module.last_request = None + module.vault_result = vault_result + module.error = error + + class ClientIdentity: + def __init__(self, name, version, kind, ka_client_version=None): + self.name = name + self.version = version + self.kind = kind + self.ka_client_version = ka_client_version + + class DeviceCredentials: + def __init__(self, encrypted_device_token, device_private_key, device_public_key=None): + self.encrypted_device_token = encrypted_device_token + self.device_private_key = device_private_key + self.device_public_key = device_public_key + + class BridgeClientConfig: + def __init__(self, server=None, region=None, socket_override=None, timeout_millis=None): + self.server = server + self.region = region + self.socket_override = socket_override + self.timeout_millis = timeout_millis + + class BootstrapRequest: + def __init__(self, client, device, flow=None, request_id=None, message_session_uid=None, + host_attestation=None, config=None): + self.client = client + self.device = device + self.flow = flow + self.request_id = request_id + self.message_session_uid = message_session_uid + self.host_attestation = host_attestation + self.config = config + + class BridgeClient(_FakeBridgeClient): + pass + + BridgeClient.module = module + module.ClientIdentity = ClientIdentity + module.DeviceCredentials = DeviceCredentials + module.BridgeClientConfig = BridgeClientConfig + module.BootstrapRequest = BootstrapRequest + module.BridgeClient = BridgeClient + return module + + +def _make_enrolled_params(): + device_private_key, _ = crypto.generate_ec_key() + device_private_key_bytes = crypto.unload_ec_private_key(device_private_key) + device_public_key = device_private_key.public_key() + params = KeeperParams(server='keepersecurity.com') + params.device_token = utils.base64_url_encode(b'encrypted-device-token') + params.device_private_key = utils.base64_url_encode(device_private_key_bytes) + params.data_key = None + return params, device_public_key + + +def _make_ka_proto_response(device_public_key, data_key): + from keepercommander.proto import APIRequest_pb2 + login_resp = APIRequest_pb2.LoginResponse() + login_resp.loginState = APIRequest_pb2.LoginState.Value('LOGGED_IN') + login_resp.encryptedSessionToken = b'ka-session-token' + login_resp.encryptedDataKey = crypto.encrypt_ec(data_key, device_public_key) + login_resp.primaryUsername = 'ka.user@example.com' + login_resp.accountUid = b'\x01' * 16 + return login_resp.SerializeToString() + + +class TestDesktopBridgeLogin(TestCase): + + def test_login_parser_accepts_via_desktop_and_rejects_new_login_conflict(self): + opts = command_utils.login_parser.parse_args(['--via-desktop']) + self.assertTrue(opts.via_desktop) + self.assertFalse(opts.new_login) + + with self.assertRaises(ParseError): + command_utils.login_parser.parse_args(['--via-desktop', '--new-login']) + + def test_command_uses_bridge_adapter_without_normal_login_fallback(self): + params = KeeperParams() + cmd = command_utils.LoginCommand() + + with mock.patch('keepercommander.auth.desktop_bridge.login_via_desktop') as bridge_login, \ + mock.patch('keepercommander.api.login') as api_login: + bridge_login.side_effect = lambda p: setattr(p, 'session_token', 'SESSION') + cmd.execute(params, via_desktop=True, skip_sync=True) + + bridge_login.assert_called_once_with(params) + api_login.assert_not_called() + self.assertTrue(params.via_desktop_login) + + def test_command_rejects_cross_level_new_login_conflict(self): + params = KeeperParams() + params.top_level_new_login = True + + with self.assertRaises(CommandError): + command_utils.LoginCommand().execute(params, via_desktop=True, skip_sync=True) + + def test_queued_shell_login_suppresses_startup_auto_login(self): + params = KeeperParams() + params.user = 'configured.user@example.com' + params.commands = ['login --via-desktop', 'q'] + + with mock.patch('keepercommander.cli.display.welcome'), \ + mock.patch('keepercommander.cli.versioning.welcome_print_version'), \ + mock.patch('keepercommander.cli.LoginCommand.execute') as login_execute: + cli.loop(params) + + login_execute.assert_called_once() + self.assertTrue(login_execute.call_args.kwargs.get('via_desktop')) + + def test_bridge_exchange_populates_in_memory_session_via_ka(self): + params, device_public_key = _make_enrolled_params() + data_key = utils.generate_aes_key() + vault_result = _VaultBootstrapResult(vault_session_token=b'vault-session-token') + bridge_module = _make_bridge_module(vault_result=vault_result) + ka_proto = _make_ka_proto_response(device_public_key, data_key) + + with mock.patch('keepercommander.loginv3.LoginV3Flow.populateAccountSummary') as populate_summary, \ + mock.patch('keepercommander.config_storage.loader.store_config_properties') as store_config, \ + mock.patch('keepercommander.rest_api.execute_rest', return_value=ka_proto): + desktop_bridge.login_via_desktop( + params, + bridge_module=bridge_module, + bridge_socket='/tmp/keeper-bridge-leaf.sock', + timeout_ms=1234, + ) + + request = bridge_module.last_request + self.assertEqual('already_enrolled', request.flow) + self.assertEqual('Keeper Commander', request.client.name) + self.assertEqual('commander', request.client.kind) + self.assertEqual(rest_api.CLIENT_VERSION, request.client.ka_client_version) + self.assertEqual('/tmp/keeper-bridge-leaf.sock', request.config.socket_override) + self.assertEqual(1234, request.config.timeout_millis) + self.assertEqual(utils.base64_url_decode(params.device_token), request.device.encrypted_device_token) + self.assertEqual(b'ka-session-token', params.session_token_bytes) + self.assertEqual(utils.base64_url_encode(b'ka-session-token'), params.session_token) + self.assertEqual(data_key, params.data_key) + self.assertIsNone(params.clone_code) + self.assertEqual('ka.user@example.com', params.user) + populate_summary.assert_called_once_with(params) + store_config.assert_not_called() + + def test_bridge_error_mapping_preserves_kdbc_fields(self): + params, _ = _make_enrolled_params() + + with self.assertRaises(desktop_bridge.DesktopBridgeLoginError) as context: + desktop_bridge.login_via_desktop(params, bridge_module=_make_bridge_module(error=_FakeBridgeError())) + + self.assertEqual('KDBC_VAULT_LOCKED', context.exception.code) + self.assertEqual('vault_locked', context.exception.kind) + self.assertTrue(context.exception.retryable) + self.assertEqual('vault', context.exception.actor) + self.assertEqual('request-1', context.exception.request_id) + + +class TestAutoEnrollment(TestCase): + + def test_fresh_install_auto_registers_device_then_proceeds(self): + """No config.json — device token absent — bridge login should auto-register.""" + params = KeeperParams(server='keepersecurity.com') + # no device_token, no device_private_key + + device_private_key_obj, _ = crypto.generate_ec_key() + device_private_key_bytes = crypto.unload_ec_private_key(device_private_key_obj) + device_public_key = device_private_key_obj.public_key() + data_key = utils.generate_aes_key() + + def fake_get_device_id(p): + p.device_token = utils.base64_url_encode(b'new-device-token') + p.device_private_key = utils.base64_url_encode(device_private_key_bytes) + + vault_result = _VaultBootstrapResult(vault_session_token=b'vault-session-token') + bridge_module = _make_bridge_module(vault_result=vault_result) + ka_proto = _make_ka_proto_response(device_public_key, data_key) + + with mock.patch('keepercommander.loginv3.LoginV3API.get_device_id', side_effect=fake_get_device_id), \ + mock.patch('keepercommander.loginv3.LoginV3Flow.populateAccountSummary'), \ + mock.patch('keepercommander.rest_api.execute_rest', return_value=ka_proto): + desktop_bridge.login_via_desktop(params, bridge_module=bridge_module) + + self.assertEqual(data_key, params.data_key) + self.assertEqual(b'ka-session-token', params.session_token_bytes) + + def test_fresh_install_registration_failure_raises_not_enrolled(self): + params = KeeperParams(server='keepersecurity.com') + + with mock.patch('keepercommander.loginv3.LoginV3API.get_device_id', + side_effect=Exception('network error')): + with self.assertRaises(desktop_bridge.DesktopBridgeLoginError) as ctx: + desktop_bridge.login_via_desktop(params, bridge_module=_make_bridge_module()) + + self.assertEqual(desktop_bridge.KDBC_CLIENT_NOT_ENROLLED, ctx.exception.code) + self.assertIn('network error', str(ctx.exception)) + + +class TestKaTransport(TestCase): + + def test_ka_login_happy_path_returns_credential_tuple(self): + device_private_key, _ = crypto.generate_ec_key() + device_public_key = device_private_key.public_key() + data_key = utils.generate_aes_key() + params = KeeperParams(server='keepersecurity.com') + + proto_bytes = _make_ka_proto_response(device_public_key, data_key) + with mock.patch('keepercommander.rest_api.execute_rest', return_value=proto_bytes): + result = desktop_bridge._ka_login_from_existing_session_token( + params, + encrypted_device_token=b'device-token', + message_session_uid=b'msg-uid', + vault_session_token=b'vault-session', + ) + + encrypted_session_token, encrypted_data_key, primary_username, account_uid = result + self.assertEqual(b'ka-session-token', encrypted_session_token) + self.assertEqual('ka.user@example.com', primary_username) + decrypted = crypto.decrypt_ec(encrypted_data_key, device_private_key) + self.assertEqual(data_key, decrypted) + + def test_ka_login_propagates_request_fields(self): + params = KeeperParams(server='keepersecurity.com') + captured = {} + + def fake_execute_rest(context, endpoint, payload, timeout=None): + captured['endpoint'] = endpoint + captured['encrypted_session_token'] = payload.encryptedSessionToken + from keepercommander.proto import APIRequest_pb2 + rq = APIRequest_pb2.StartLoginRequest() + rq.ParseFromString(payload.payload) + captured['encrypted_device_token'] = rq.encryptedDeviceToken + captured['message_session_uid'] = rq.messageSessionUid + return {'error': 'test_stop', 'message': 'test_stop'} + + with self.assertRaises(desktop_bridge.DesktopBridgeLoginError): + with mock.patch('keepercommander.rest_api.execute_rest', side_effect=fake_execute_rest): + desktop_bridge._ka_login_from_existing_session_token( + params, + encrypted_device_token=b'my-device-token', + message_session_uid=b'my-msg-uid', + vault_session_token=b'my-vault-session', + ) + + self.assertEqual('authentication/login_from_existing_session_token', captured['endpoint']) + self.assertEqual(b'my-device-token', captured['encrypted_device_token']) + self.assertEqual(b'my-msg-uid', captured['message_session_uid']) + self.assertEqual(b'my-vault-session', captured['encrypted_session_token']) + + def test_ka_login_raises_on_dict_error_response(self): + params = KeeperParams(server='keepersecurity.com') + with mock.patch('keepercommander.rest_api.execute_rest', + return_value={'error': 'invalid_device', 'message': 'device rejected'}): + with self.assertRaises(desktop_bridge.DesktopBridgeLoginError) as ctx: + desktop_bridge._ka_login_from_existing_session_token( + params, b'tok', b'uid', b'vsession', + ) + self.assertEqual(desktop_bridge.KDBC_KA_LOGIN_FAILED, ctx.exception.code) + self.assertIn('device rejected', str(ctx.exception)) + + def test_ka_login_raises_on_unexpected_login_state(self): + from keepercommander.proto import APIRequest_pb2 + login_resp = APIRequest_pb2.LoginResponse() + login_resp.loginState = APIRequest_pb2.LoginState.Value('REQUIRES_2FA') + params = KeeperParams(server='keepersecurity.com') + with mock.patch('keepercommander.rest_api.execute_rest', + return_value=login_resp.SerializeToString()): + with self.assertRaises(desktop_bridge.DesktopBridgeLoginError) as ctx: + desktop_bridge._ka_login_from_existing_session_token( + params, b'tok', b'uid', b'vsession', + ) + self.assertEqual(desktop_bridge.KDBC_KA_LOGIN_FAILED, ctx.exception.code) From 6be65e4a4aecd11392dd37f64dab1429a8c67b82 Mon Sep 17 00:00:00 2001 From: Ayrris Aunario <105313137+aaunario-keeper@users.noreply.github.com> Date: Thu, 11 Jun 2026 19:21:54 -0500 Subject: [PATCH 7/8] fix: allow --via-desktop with --new-login, remove false conflict guard --new-login (force fresh auth) and --via-desktop (auth method) are orthogonal. Removed mutual exclusion from login_parser, __main__.py, and LoginCommand.execute. Also removes top_level_new_login from params since it only existed to enforce the now-removed restriction. --- keepercommander/__main__.py | 5 ----- keepercommander/commands/utils.py | 11 +++-------- keepercommander/params.py | 1 - unit-tests/test_desktop_bridge_login.py | 14 ++++---------- 4 files changed, 7 insertions(+), 24 deletions(-) diff --git a/keepercommander/__main__.py b/keepercommander/__main__.py index 7b9dbe35a..3f5bfb02a 100644 --- a/keepercommander/__main__.py +++ b/keepercommander/__main__.py @@ -206,10 +206,6 @@ def main(from_package=False): sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0]) opts, flags = parser.parse_known_args(sys.argv[1:]) - if opts.via_desktop and opts.new_login: - logging.error('--via-desktop cannot be used with --new-login') - sys.exit(1) - # Store the original command arguments for proper reconstruction if opts.command: # Find where the command starts in the original args and take everything after it @@ -305,7 +301,6 @@ def main(from_package=False): params.rest_context.fail_on_throttle = opts.fail_on_throttle params.via_desktop_login = opts.via_desktop - params.top_level_new_login = opts.new_login if opts.password: params.password = opts.password diff --git a/keepercommander/commands/utils.py b/keepercommander/commands/utils.py index 6c9ecbbc6..a745eee0c 100644 --- a/keepercommander/commands/utils.py +++ b/keepercommander/commands/utils.py @@ -294,10 +294,9 @@ def register_command_info(aliases, command_info): login_parser = argparse.ArgumentParser(prog='login', description='Start a login session on Commander') login_parser.add_argument('-p', '--pass', dest='password', action='store', help='master password') -login_mode_group = login_parser.add_mutually_exclusive_group() -login_mode_group.add_argument('--new-login', dest='new_login', action='store_true', help='Force full login flow') -login_mode_group.add_argument('--via-desktop', dest='via_desktop', action='store_true', - help='Use Keeper Desktop bridge for this login') +login_parser.add_argument('--new-login', dest='new_login', action='store_true', help='Force full login flow') +login_parser.add_argument('--via-desktop', dest='via_desktop', action='store_true', + help='Use Keeper Desktop bridge for this login') login_parser.add_argument('--server', dest='server', action='store', help='Data center region (US, EU, AU, CA, JP, GOV, etc.)') login_parser.add_argument('--config-file', dest='config_file', action='store_true', help='Store config in config.json instead of the OS-native keychain ' @@ -1696,10 +1695,6 @@ def execute(self, params, **kwargs): new_login = kwargs.get('new_login') is True skip_sync = kwargs.get('skip_sync') is True via_desktop = kwargs.get('via_desktop') is True or getattr(params, 'via_desktop_login', False) is True - if via_desktop and new_login: - raise CommandError('', '--via-desktop cannot be used with --new-login') - if kwargs.get('via_desktop') is True and getattr(params, 'top_level_new_login', False): - raise CommandError('', '--via-desktop cannot be used with --new-login') if kwargs.get('via_desktop') is True: params.via_desktop_login = True diff --git a/keepercommander/params.py b/keepercommander/params.py index 3a9fbc0c5..5b9008fd4 100644 --- a/keepercommander/params.py +++ b/keepercommander/params.py @@ -244,7 +244,6 @@ def __init__(self, config_filename='', config=None, server='keepersecurity.com') self.iterations = 0 self.biometric = None self.via_desktop_login = False - self.top_level_new_login = False self.service_mode = False # Flag to indicate if running in service mode self.thread_local = threading.local() self._pedm_plugin = None # type: diff --git a/unit-tests/test_desktop_bridge_login.py b/unit-tests/test_desktop_bridge_login.py index 41502f5cb..6b0975b40 100644 --- a/unit-tests/test_desktop_bridge_login.py +++ b/unit-tests/test_desktop_bridge_login.py @@ -191,13 +191,14 @@ def _make_ka_proto_response(device_public_key, data_key): class TestDesktopBridgeLogin(TestCase): - def test_login_parser_accepts_via_desktop_and_rejects_new_login_conflict(self): + def test_login_parser_accepts_via_desktop_and_new_login_together(self): opts = command_utils.login_parser.parse_args(['--via-desktop']) self.assertTrue(opts.via_desktop) self.assertFalse(opts.new_login) - with self.assertRaises(ParseError): - command_utils.login_parser.parse_args(['--via-desktop', '--new-login']) + opts = command_utils.login_parser.parse_args(['--via-desktop', '--new-login']) + self.assertTrue(opts.via_desktop) + self.assertTrue(opts.new_login) def test_command_uses_bridge_adapter_without_normal_login_fallback(self): params = KeeperParams() @@ -212,13 +213,6 @@ def test_command_uses_bridge_adapter_without_normal_login_fallback(self): api_login.assert_not_called() self.assertTrue(params.via_desktop_login) - def test_command_rejects_cross_level_new_login_conflict(self): - params = KeeperParams() - params.top_level_new_login = True - - with self.assertRaises(CommandError): - command_utils.LoginCommand().execute(params, via_desktop=True, skip_sync=True) - def test_queued_shell_login_suppresses_startup_auto_login(self): params = KeeperParams() params.user = 'configured.user@example.com' From 883df4014a7536e98f3c433dedbfbecdf1e799d2 Mon Sep 17 00:00:00 2001 From: Ayrris Aunario <105313137+aaunario-keeper@users.noreply.github.com> Date: Sun, 14 Jun 2026 16:22:04 -0500 Subject: [PATCH 8/8] fix: auto desktop bridge login for dev shells --- keepercommander/auth/desktop_bridge.py | 79 +++++++++++++++++++++---- keepercommander/cli.py | 11 +++- unit-tests/test_desktop_bridge_login.py | 74 ++++++++++++++++++++++- 3 files changed, 148 insertions(+), 16 deletions(-) diff --git a/keepercommander/auth/desktop_bridge.py b/keepercommander/auth/desktop_bridge.py index c21e3906a..61a2dca42 100644 --- a/keepercommander/auth/desktop_bridge.py +++ b/keepercommander/auth/desktop_bridge.py @@ -12,13 +12,16 @@ import importlib import logging +import os import uuid +from urllib.parse import urlparse from .. import __version__, crypto, rest_api, utils from ..params import KeeperParams DEFAULT_BRIDGE_TIMEOUT_MS = 160000 +DEV_BRIDGE_VERIFICATION_POLICY = 'log_only' KDBC_CLIENT_NOT_ENROLLED = 'KDBC_CLIENT_NOT_ENROLLED' KDBC_KA_LOGIN_FAILED = 'KDBC_KA_LOGIN_FAILED' KDBC_PROTOCOL_ERROR = 'KDBC_PROTOCOL_ERROR' @@ -37,19 +40,26 @@ def __init__(self, message, code=KDBC_INTERNAL_ERROR, kind='internal_error', ret self.request_id = request_id def __str__(self): - parts = [f'Desktop bridge login failed [{self.code}]: {self.message}'] - if self.retryable: - parts.append('The operation may be retried after the reported condition is resolved.') + parts = [ + 'Desktop bridge login failed:', + f'code={self.code}', + f'kind={self.kind}', + f'actor={self.actor}', + f'retryable={self.retryable}', + ] if self.request_id: parts.append(f'request_id={self.request_id}') + parts.append(f'message={self.message}') return ' '.join(parts) -def login_via_desktop(params, bridge_module=None, bridge_socket=None, timeout_ms=DEFAULT_BRIDGE_TIMEOUT_MS): +def login_via_desktop(params, bridge_module=None, bridge_socket=None, timeout_ms=DEFAULT_BRIDGE_TIMEOUT_MS, + verification_policy=None): # type: (KeeperParams, object, str, int) -> None module = bridge_module or _load_bridge_module() device_token, device_private_key = _get_device_credentials(params) - request = _build_bootstrap_request(module, params, device_token, device_private_key, bridge_socket, timeout_ms) + request = _build_bootstrap_request( + module, params, device_token, device_private_key, bridge_socket, timeout_ms, verification_policy) try: vault_result = module.BridgeClient().exchange_vault_token(request) @@ -198,7 +208,8 @@ def _get_device_credentials(params): return device_token, device_private_key -def _build_bootstrap_request(module, params, device_token, device_private_key, bridge_socket, timeout_ms): +def _build_bootstrap_request(module, params, device_token, device_private_key, bridge_socket, timeout_ms, + verification_policy): private_key = crypto.load_ec_private_key(device_private_key) device_public_key = crypto.unload_ec_public_key(private_key.public_key()) client = module.ClientIdentity( @@ -213,13 +224,7 @@ def _build_bootstrap_request(module, params, device_token, device_private_key, b device_public_key=device_public_key, ) - config = None - if hasattr(module, 'BridgeClientConfig'): - config = module.BridgeClientConfig( - server=params.server, - socket_override=bridge_socket, - timeout_millis=timeout_ms, - ) + config = _build_bridge_config(module, params, bridge_socket, timeout_ms, verification_policy) return module.BootstrapRequest( client=client, @@ -231,6 +236,54 @@ def _build_bootstrap_request(module, params, device_token, device_private_key, b ) +def _build_bridge_config(module, params, bridge_socket, timeout_ms, verification_policy): + if not hasattr(module, 'BridgeClientConfig'): + return None + + verification_policy = _resolve_verification_policy(params, verification_policy) + kwargs = { + 'server': params.server, + 'socket_override': bridge_socket, + 'timeout_millis': timeout_ms, + } + if verification_policy: + kwargs['verification_policy'] = verification_policy + + try: + return module.BridgeClientConfig(**kwargs) + except TypeError: + # Older KDBC wheels do not expose verification_policy. + kwargs.pop('verification_policy', None) + return module.BridgeClientConfig(**kwargs) + + +def _resolve_verification_policy(params, verification_policy): + if verification_policy: + return verification_policy + + env_policy = os.environ.get('KDBC_VERIFICATION_POLICY') + if env_policy: + return env_policy + + if _is_keeper_dev_host(getattr(params, 'server', '')): + return DEV_BRIDGE_VERIFICATION_POLICY + + return None + + +def _is_keeper_dev_host(server): + if not server: + return False + + server = str(server).strip().lower() + if not server: + return False + + parsed = urlparse(server if '://' in server else f'https://{server}') + host = parsed.hostname or '' + return host.startswith('dev.keepersecurity.') or host.startswith('govcloud.dev.keepersecurity.') + + def _translate_bridge_error(exc): code = getattr(exc, 'code', None) or KDBC_INTERNAL_ERROR kind = getattr(exc, 'kind', None) or 'bridge_error' diff --git a/keepercommander/cli.py b/keepercommander/cli.py index ff23c56b4..5628ea1d4 100644 --- a/keepercommander/cli.py +++ b/keepercommander/cli.py @@ -794,9 +794,16 @@ def _(event): display.show_government_warning() if not params.batch_mode and not skip_init: - if params.user and not _first_queued_command_is_login(params): + startup_via_desktop = getattr(params, 'via_desktop_login', False) is True + if (params.user or startup_via_desktop) and not _first_queued_command_is_login(params): try: - LoginCommand().execute(params, email=params.user, password=params.password, new_login=new_login) + LoginCommand().execute( + params, + email=params.user, + password=params.password, + new_login=new_login, + via_desktop=startup_via_desktop, + ) except KeyboardInterrupt: logging.info('') except EOFError: diff --git a/unit-tests/test_desktop_bridge_login.py b/unit-tests/test_desktop_bridge_login.py index 6b0975b40..5da406ab7 100644 --- a/unit-tests/test_desktop_bridge_login.py +++ b/unit-tests/test_desktop_bridge_login.py @@ -138,11 +138,13 @@ def __init__(self, encrypted_device_token, device_private_key, device_public_key self.device_public_key = device_public_key class BridgeClientConfig: - def __init__(self, server=None, region=None, socket_override=None, timeout_millis=None): + def __init__(self, server=None, region=None, socket_override=None, timeout_millis=None, + verification_policy=None): self.server = server self.region = region self.socket_override = socket_override self.timeout_millis = timeout_millis + self.verification_policy = verification_policy class BootstrapRequest: def __init__(self, client, device, flow=None, request_id=None, message_session_uid=None, @@ -226,6 +228,20 @@ def test_queued_shell_login_suppresses_startup_auto_login(self): login_execute.assert_called_once() self.assertTrue(login_execute.call_args.kwargs.get('via_desktop')) + def test_shell_startup_uses_session_wide_via_desktop_without_login_command(self): + params = KeeperParams() + params.via_desktop_login = True + params.commands = ['q'] + + with mock.patch('keepercommander.cli.display.welcome'), \ + mock.patch('keepercommander.cli.versioning.welcome_print_version'), \ + mock.patch('keepercommander.cli.LoginCommand.execute') as login_execute: + cli.loop(params) + + login_execute.assert_called_once() + self.assertTrue(login_execute.call_args.kwargs.get('via_desktop')) + self.assertEqual('', login_execute.call_args.kwargs.get('email')) + def test_bridge_exchange_populates_in_memory_session_via_ka(self): params, device_public_key = _make_enrolled_params() data_key = utils.generate_aes_key() @@ -241,6 +257,7 @@ def test_bridge_exchange_populates_in_memory_session_via_ka(self): bridge_module=bridge_module, bridge_socket='/tmp/keeper-bridge-leaf.sock', timeout_ms=1234, + verification_policy='log_only', ) request = bridge_module.last_request @@ -250,6 +267,7 @@ def test_bridge_exchange_populates_in_memory_session_via_ka(self): self.assertEqual(rest_api.CLIENT_VERSION, request.client.ka_client_version) self.assertEqual('/tmp/keeper-bridge-leaf.sock', request.config.socket_override) self.assertEqual(1234, request.config.timeout_millis) + self.assertEqual('log_only', request.config.verification_policy) self.assertEqual(utils.base64_url_decode(params.device_token), request.device.encrypted_device_token) self.assertEqual(b'ka-session-token', params.session_token_bytes) self.assertEqual(utils.base64_url_encode(b'ka-session-token'), params.session_token) @@ -270,6 +288,60 @@ def test_bridge_error_mapping_preserves_kdbc_fields(self): self.assertTrue(context.exception.retryable) self.assertEqual('vault', context.exception.actor) self.assertEqual('request-1', context.exception.request_id) + self.assertIn('code=KDBC_VAULT_LOCKED', str(context.exception)) + self.assertIn('kind=vault_locked', str(context.exception)) + self.assertIn('actor=vault', str(context.exception)) + self.assertIn('retryable=True', str(context.exception)) + + def test_bridge_config_uses_verification_policy_from_environment(self): + params, _ = _make_enrolled_params() + bridge_module = _make_bridge_module(vault_result=_VaultBootstrapResult(b'vault-session-token')) + + with mock.patch.dict('os.environ', {'KDBC_VERIFICATION_POLICY': 'log_only'}): + request = desktop_bridge._build_bootstrap_request( + bridge_module, + params, + utils.base64_url_decode(params.device_token), + utils.base64_url_decode(params.device_private_key), + None, + 1234, + None, + ) + + self.assertEqual('log_only', request.config.verification_policy) + + def test_bridge_config_defaults_dev_hosts_to_log_only(self): + params = KeeperParams(server='dev.keepersecurity.com') + bridge_module = _make_bridge_module(vault_result=_VaultBootstrapResult(b'vault-session-token')) + + with mock.patch.dict('os.environ', {}, clear=True): + config = desktop_bridge._build_bridge_config( + bridge_module, params, None, 1234, None, + ) + + self.assertEqual('log_only', config.verification_policy) + + def test_bridge_config_leaves_production_policy_to_kdbc_default(self): + params = KeeperParams(server='keepersecurity.com') + bridge_module = _make_bridge_module(vault_result=_VaultBootstrapResult(b'vault-session-token')) + + with mock.patch.dict('os.environ', {}, clear=True): + config = desktop_bridge._build_bridge_config( + bridge_module, params, None, 1234, None, + ) + + self.assertIsNone(config.verification_policy) + + def test_bridge_config_environment_overrides_dev_default(self): + params = KeeperParams(server='dev.keepersecurity.com') + bridge_module = _make_bridge_module(vault_result=_VaultBootstrapResult(b'vault-session-token')) + + with mock.patch.dict('os.environ', {'KDBC_VERIFICATION_POLICY': 'enforce'}, clear=True): + config = desktop_bridge._build_bridge_config( + bridge_module, params, None, 1234, None, + ) + + self.assertEqual('enforce', config.verification_policy) class TestAutoEnrollment(TestCase):