From 0e29a6e0baeb1d279f88d970b65c7da2bb9d096c Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Mon, 4 May 2026 11:56:16 +0530 Subject: [PATCH 1/7] Implement workflow recent changes after apr 22 --- .../commands/workflow/config_commands.py | 8 +- keepercommander/commands/workflow/helpers.py | 109 +++++++++++++----- keepercommander/commands/workflow/mfa.py | 23 +--- keepercommander/commands/workflow/registry.py | 3 +- 4 files changed, 89 insertions(+), 54 deletions(-) diff --git a/keepercommander/commands/workflow/config_commands.py b/keepercommander/commands/workflow/config_commands.py index 28ca9c71f..360c41f6c 100644 --- a/keepercommander/commands/workflow/config_commands.py +++ b/keepercommander/commands/workflow/config_commands.py @@ -83,8 +83,6 @@ class WorkflowCreateCommand(Command): help='Comma-separated allowed days (e.g., "mon,tue,wed,thu,fri")') parser.add_argument('--time-range', type=str, help='Allowed time range in HH:MM-HH:MM format (e.g., "09:00-17:00")') - parser.add_argument('--timezone', type=str, - help='Timezone for allowed times (e.g., "America/New_York")') parser.add_argument('-u', '--approver', action='append', help='User email to add as an approver. Pass multiple times to ' 'add several. Required when --approvals-needed > 0. ' @@ -155,7 +153,7 @@ def execute(self, params: KeeperParams, **kwargs): parameters.accessLength = WorkflowFormatter.parse_duration(kwargs.get('duration', '1d')) temporal_filter = WorkflowFormatter.build_temporal_filter( - kwargs.get('allowed_days'), kwargs.get('time_range'), kwargs.get('timezone'), + kwargs.get('allowed_days'), kwargs.get('time_range'), ) if temporal_filter: parameters.allowedTimes.CopyFrom(temporal_filter) @@ -386,8 +384,6 @@ class WorkflowUpdateCommand(Command): help='Comma-separated allowed days (e.g., "mon,tue,wed,thu,fri")') parser.add_argument('--time-range', type=str, help='Allowed time range in HH:MM-HH:MM format (e.g., "09:00-17:00")') - parser.add_argument('--timezone', type=str, - help='Timezone for allowed times (e.g., "America/New_York")') parser.add_argument('--format', dest='format', action='store', choices=['table', 'json'], default='table', help='Output format') @@ -434,7 +430,7 @@ def execute(self, params: KeeperParams, **kwargs): updates_provided = True temporal_filter = WorkflowFormatter.build_temporal_filter( - kwargs.get('allowed_days'), kwargs.get('time_range'), kwargs.get('timezone'), + kwargs.get('allowed_days'), kwargs.get('time_range'), ) if temporal_filter: parameters.allowedTimes.CopyFrom(temporal_filter) diff --git a/keepercommander/commands/workflow/helpers.py b/keepercommander/commands/workflow/helpers.py index 21a3ac4dd..7cf2e55da 100644 --- a/keepercommander/commands/workflow/helpers.py +++ b/keepercommander/commands/workflow/helpers.py @@ -32,9 +32,6 @@ def sanitize_router_error(error: Exception) -> str: return msg or 'Unknown error' -_ENFORCEMENT_KEY = 'allow_configure_workflow_settings' - - def print_exempt_message(fmt='table'): """Print the standard exemption message in the appropriate format.""" import json as _json @@ -42,41 +39,66 @@ def print_exempt_message(fmt='table'): if fmt == 'json': print(_json.dumps({'status': 'exempt', 'message': 'Workflow not required'}, indent=2)) else: - print(f"\n{_bc.WARNING}You have edit access and workflow management permissions for this record.{_bc.ENDC}\n") - print("Workflow is not required — you can access this resource directly.\n") - + print(f"\n{_bc.WARNING}You are exempt from workflow restrictions on this record.{_bc.ENDC}") + print("As a record owner or approver, you can access this resource directly.\n") -def is_workflow_exempt(params, record_uid): - """Users with edit access AND 'Can manage workflow settings' are exempt from workflow.""" - enforcements = getattr(params, 'enforcements', None) - if not enforcements or 'booleans' not in enforcements: - return False - can_manage = any( - b.get('value') for b in enforcements['booleans'] - if b.get('key') == _ENFORCEMENT_KEY - ) - if not can_manage: - return False +def is_record_owner(params, record_uid): + """Check if the current user is the owner of the given record.""" if record_uid in getattr(params, 'record_owner_cache', {}): owner_info = params.record_owner_cache[record_uid] if getattr(owner_info, 'owner', False): return True + return False - meta = getattr(params, 'meta_data_cache', {}).get(record_uid) - if meta and meta.get('can_edit'): - return True - for sf_uid in getattr(params, 'shared_folder_cache', {}): - sf = params.shared_folder_cache[sf_uid] - for sfr in sf.get('records', []): - if sfr.get('record_uid') == record_uid: - if sfr.get('owner') or sfr.get('can_edit'): - return True +def is_on_approver_list(params, config): + """Check if the current user appears on the workflow config's approver list, + either directly by email or via team membership.""" + if not config or not config.approvers: + return False + + current_user = getattr(params, 'user', '') + team_cache = getattr(params, 'team_cache', {}) + for approver in config.approvers: + if approver.user and approver.user.lower() == current_user.lower(): + return True + if approver.teamUid: + team_uid_b64 = utils.base64_url_encode(approver.teamUid) + if team_uid_b64 in team_cache: + return True return False +def is_workflow_exempt(params, record_uid, config=None): + """Exempt users: record owner OR on the approver list. + + If *config* is already available (e.g. from a prior read_workflow_config + call) pass it to avoid an extra API round-trip. When *config* is None the + function reads it from the router; a transport failure is treated as + non-exempt (fail closed). + """ + if is_record_owner(params, record_uid): + return True + + if config is None: + from ..pam.router_helper import _post_request_to_router + try: + ref = ProtobufRefBuilder.record_ref( + utils.base64_url_decode(record_uid), + '', + ) + config = _post_request_to_router( + params, 'read_workflow_config', + rq_proto=ref, rs_type=workflow_pb2.WorkflowConfig, + ) + except Exception: + return False + + return is_on_approver_list(params, config) + + def is_pam_action_allowed_by_enforcement(params: KeeperParams, enforcement_key: str) -> bool: """Per-user enterprise enforcement gate: is this user permitted to perform the action by their enterprise enforcement profile? @@ -490,8 +512,8 @@ def format_duration(milliseconds: int) -> str: return f"{seconds} second{'s' if seconds != 1 else ''}" @staticmethod - def build_temporal_filter(allowed_days_str, time_range_str, timezone_str): - if not allowed_days_str and not time_range_str and not timezone_str: + def build_temporal_filter(allowed_days_str, time_range_str): + if not allowed_days_str and not time_range_str: return None temporal = workflow_pb2.TemporalAccessFilter() @@ -516,11 +538,38 @@ def build_temporal_filter(allowed_days_str, time_range_str, timezone_str): time_range.endTime = end_hhmm temporal.timeRanges.append(time_range) - if timezone_str: - temporal.timeZone = timezone_str + temporal.timeZone = WorkflowFormatter._get_local_iana_timezone() return temporal + @staticmethod + def _get_local_iana_timezone(): + import os + + tz = os.environ.get('TZ') + if tz and '/' in tz: + return tz + + try: + link = os.readlink('/etc/localtime') + marker = '/zoneinfo/' + idx = link.find(marker) + if idx != -1: + return link[idx + len(marker):] + except (OSError, ValueError): + pass + + try: + with open('/etc/timezone', 'r') as f: + tz = f.read().strip() + if tz and '/' in tz: + return tz + except (OSError, IOError): + pass + + raise CommandError('', 'Could not detect local IANA timezone. ' + 'Set the TZ environment variable (e.g., TZ=Asia/Kolkata)') + @staticmethod def _parse_time_to_hhmm(time_str): """Parse 'HH:MM' to the HHMM integer the server stores on diff --git a/keepercommander/commands/workflow/mfa.py b/keepercommander/commands/workflow/mfa.py index d9a977eac..3f621208a 100644 --- a/keepercommander/commands/workflow/mfa.py +++ b/keepercommander/commands/workflow/mfa.py @@ -24,7 +24,8 @@ ProtobufRefBuilder, WorkflowFormatter, is_gateway_online_for_record, - is_workflow_exempt, + is_on_approver_list, + is_record_owner, prompt_for_reason_ticket, sanitize_router_error, start_workflow_for_record, @@ -68,24 +69,9 @@ def __init__(self, params: KeeperParams, record_uid: str): self.record_name = record.title if record else record_uid def validate(self, silent_actionable: bool = False) -> dict: - if is_workflow_exempt(self.params, self.record_uid): + if is_record_owner(self.params, self.record_uid): return dict(self._DEFAULT_RESULT) - # Workflow REST endpoints (`read_workflow_config`, - # `get_user_access_state`, `get_workflow_state`) are not yet deployed - # on every router. On a router that doesn't expose them, the call - # raises (404 / unsupported / RRC error) and `_post_request_to_router` - # bubbles it up; the wrappers below convert that into _TRANSPORT_ERROR. - # We can't tell from the wire whether the failure is "endpoint not - # deployed" (prod today) or "endpoint deployed but momentarily - # unreachable" (transient QA hiccup). Erring on the side of legacy - # compatibility: treat _TRANSPORT_ERROR as "no workflow protection - # on this record, defer to the gateway." The gateway is the - # authoritative gate on prod; on QA the workflow service still - # enforces server-side, so a flaky read just relaxes the *client* - # gate without opening a real security gap. Old behavior (block - # with a banner) was correct for QA but a hard regression on prod - # legacy launches that have never seen workflow. config = self._read_workflow_config() if config is _TRANSPORT_ERROR: logging.debug( @@ -97,6 +83,9 @@ def validate(self, silent_actionable: bool = False) -> dict: if config is None: return dict(self._DEFAULT_RESULT) + if is_on_approver_list(self.params, config): + return dict(self._DEFAULT_RESULT) + mfa_required = bool(config.parameters and config.parameters.requireMFA) if not self._check_allowed_times(config): diff --git a/keepercommander/commands/workflow/registry.py b/keepercommander/commands/workflow/registry.py index 2ea6f31ed..5353dfa52 100644 --- a/keepercommander/commands/workflow/registry.py +++ b/keepercommander/commands/workflow/registry.py @@ -11,7 +11,8 @@ from ..base import GroupCommand, dump_report_data from ...display import bcolors -from .helpers import _ENFORCEMENT_KEY + +_ENFORCEMENT_KEY = 'allow_configure_workflow_settings' from .config_commands import ( WorkflowCreateCommand, From 8dea82ab903765cfa487e7ae9beec8001b9766be Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Mon, 4 May 2026 12:27:40 +0530 Subject: [PATCH 2/7] Remove flow uid support in state --- keepercommander/commands/workflow/mfa.py | 3 +-- .../commands/workflow/state_commands.py | 26 ++++++------------- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/keepercommander/commands/workflow/mfa.py b/keepercommander/commands/workflow/mfa.py index 3f621208a..c7f73168b 100644 --- a/keepercommander/commands/workflow/mfa.py +++ b/keepercommander/commands/workflow/mfa.py @@ -312,8 +312,7 @@ def _print_needs_action(self, conditions, flow_uid_bytes): cond_str = WorkflowFormatter.format_conditions(conditions) print(f"Pending conditions: {cond_str}") elif flow_uid_bytes: - flow_uid_str = utils.base64_url_encode(flow_uid_bytes) - print(f"Run: {bcolors.OKBLUE}pam workflow state --flow-uid {flow_uid_str}{bcolors.ENDC} " + print(f"Run: {bcolors.OKBLUE}pam workflow state {self.record_name}{bcolors.ENDC} " f"to see details.") print() diff --git a/keepercommander/commands/workflow/state_commands.py b/keepercommander/commands/workflow/state_commands.py index 751b7c8c4..6505a3132 100644 --- a/keepercommander/commands/workflow/state_commands.py +++ b/keepercommander/commands/workflow/state_commands.py @@ -35,11 +35,9 @@ def _fmt_ts_or_empty(ts_ms: int) -> str: class WorkflowGetStateCommand(Command): parser = argparse.ArgumentParser( prog='pam workflow state', - description='Get workflow state for a record or flow', + description='Get workflow state for a record', ) - _state_group = parser.add_mutually_exclusive_group(required=True) - _state_group.add_argument('-r', '--record', help='Record UID or name') - _state_group.add_argument('-f', '--flow-uid', help='Flow UID of active workflow') + parser.add_argument('record', help='Record UID or name') parser.add_argument('--format', dest='format', action='store', choices=['table', 'json'], default='table', help='Output format') @@ -47,22 +45,14 @@ def get_parser(self): return WorkflowGetStateCommand.parser def execute(self, params: KeeperParams, **kwargs): - record_uid = kwargs.get('record') - flow_uid = kwargs.get('flow_uid') + record_uid, record = RecordResolver.resolve(params, kwargs.get('record')) + if is_workflow_exempt(params, record_uid): + print_exempt_message(kwargs.get('format', 'table')) + return state = workflow_pb2.WorkflowState() - if flow_uid: - try: - state.flowUid = utils.base64_url_decode(flow_uid) - except Exception: - raise CommandError('', f'Invalid flow UID: "{flow_uid}"') - else: - record_uid, record = RecordResolver.resolve(params, record_uid) - if is_workflow_exempt(params, record_uid): - print_exempt_message(kwargs.get('format', 'table')) - return - record_uid_bytes = utils.base64_url_decode(record_uid) - state.resource.CopyFrom(ProtobufRefBuilder.record_ref(record_uid_bytes, record.title)) + record_uid_bytes = utils.base64_url_decode(record_uid) + state.resource.CopyFrom(ProtobufRefBuilder.record_ref(record_uid_bytes, record.title)) try: response = _post_request_to_router( From 51b40ec0c9dba44d631736b8de636a546ca06680 Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Tue, 5 May 2026 11:06:09 +0530 Subject: [PATCH 3/7] Remove column from my-access and fix mfa prompts --- keepercommander/commands/workflow/mfa.py | 16 ++++++++++------ .../commands/workflow/state_commands.py | 5 ++--- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/keepercommander/commands/workflow/mfa.py b/keepercommander/commands/workflow/mfa.py index c7f73168b..ac883de76 100644 --- a/keepercommander/commands/workflow/mfa.py +++ b/keepercommander/commands/workflow/mfa.py @@ -355,6 +355,13 @@ def prompt(self): from ... import api tfa_list = self._fetch_2fa_list(self.params, api, APIRequest_pb2) + if tfa_list is self._NO_2FA_CONFIGURED: + print(f"\n{bcolors.FAIL}This workflow requires 2FA verification{bcolors.ENDC}") + print( + "Your account does not have any 2FA methods configured. " + f"For available methods, run: {bcolors.OKBLUE}2fa add -h{bcolors.ENDC}\n" + ) + return None if tfa_list is None: try: code = getpass.getpass('2FA required. Enter TOTP code: ').strip() @@ -382,6 +389,8 @@ def prompt(self): return self._dispatch(selected.channelType, APIRequest_pb2) + _NO_2FA_CONFIGURED = object() + @staticmethod def _fetch_2fa_list(params, api, APIRequest_pb2): try: @@ -393,12 +402,7 @@ def _fetch_2fa_list(params, api, APIRequest_pb2): return None if not tfa_list.channels: - print(f"\n{bcolors.FAIL}This workflow requires 2FA verification{bcolors.ENDC}") - print( - "Your account does not have any 2FA methods configured. " - f"For available methods, run: {bcolors.OKBLUE}2fa add -h{bcolors.ENDC}" - ) - return None + return WorkflowMfaPrompt._NO_2FA_CONFIGURED return tfa_list diff --git a/keepercommander/commands/workflow/state_commands.py b/keepercommander/commands/workflow/state_commands.py index 6505a3132..459007da7 100644 --- a/keepercommander/commands/workflow/state_commands.py +++ b/keepercommander/commands/workflow/state_commands.py @@ -194,7 +194,6 @@ def _print_table(params, response): record_name = RecordResolver.resolve_name(params, wf.resource) record_uid = utils.base64_url_encode(wf.resource.value) if wf.resource.value else '' flow_uid = utils.base64_url_encode(wf.flowUid) if wf.flowUid else '' - conditions = WorkflowFormatter.format_conditions(wf.status.conditions) if wf.status.conditions else '' checked_out_by = wf.status.checkedOutBy or '' started = _fmt_ts_or_empty(wf.status.startedOn) expires = _fmt_ts_or_empty(wf.status.expiresOn) @@ -205,9 +204,9 @@ def _print_table(params, response): for a in wf.status.approvedBy ] approved_by = ', '.join(approved_names) - rows.append([stage, record_name, record_uid, flow_uid, checked_out_by, approved_by, started, expires, conditions]) + rows.append([stage, record_name, record_uid, flow_uid, checked_out_by, approved_by, started, expires]) - headers = ['Stage', 'Record Name', 'Record UID', 'Flow UID', 'Checked Out By', 'Approved By', 'Started', 'Expires', 'Conditions'] + headers = ['Stage', 'Record Name', 'Record UID', 'Flow UID', 'Checked Out By', 'Approved By', 'Started', 'Expires'] print() dump_report_data(rows, headers=headers) print() From 6b9aeeae1bb530258e4cc55a0b66389810ff53ea Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Tue, 5 May 2026 12:08:02 +0530 Subject: [PATCH 4/7] Fix flow uids starting with - and my-access table view --- .../commands/workflow/approver_commands.py | 10 +++++- keepercommander/commands/workflow/helpers.py | 36 +++++++++++++++++++ .../commands/workflow/requester_commands.py | 9 +++++ .../commands/workflow/state_commands.py | 2 +- 4 files changed, 55 insertions(+), 2 deletions(-) diff --git a/keepercommander/commands/workflow/approver_commands.py b/keepercommander/commands/workflow/approver_commands.py index 342579714..2b04a6e5f 100644 --- a/keepercommander/commands/workflow/approver_commands.py +++ b/keepercommander/commands/workflow/approver_commands.py @@ -22,7 +22,7 @@ from ...proto import workflow_pb2 from ... import api, crypto, utils -from .helpers import RecordResolver, WorkflowFormatter, sanitize_router_error +from .helpers import RecordResolver, WorkflowFormatter, sanitize_router_error, fix_dash_uid_args class WorkflowGetApprovalRequestsCommand(Command): @@ -183,6 +183,10 @@ class WorkflowApproveCommand(Command): def get_parser(self): return WorkflowApproveCommand.parser + def execute_args(self, params, args, **kwargs): + args = fix_dash_uid_args(self.get_parser(), args) + return super().execute_args(params, args, **kwargs) + def execute(self, params: KeeperParams, **kwargs): flow_uid = kwargs.get('flow_uid') try: @@ -222,6 +226,10 @@ class WorkflowDenyCommand(Command): def get_parser(self): return WorkflowDenyCommand.parser + def execute_args(self, params, args, **kwargs): + args = fix_dash_uid_args(self.get_parser(), args) + return super().execute_args(params, args, **kwargs) + def execute(self, params: KeeperParams, **kwargs): flow_uid = kwargs.get('flow_uid') reason = kwargs.get('reason') or '' diff --git a/keepercommander/commands/workflow/helpers.py b/keepercommander/commands/workflow/helpers.py index 7cf2e55da..6f2c586d9 100644 --- a/keepercommander/commands/workflow/helpers.py +++ b/keepercommander/commands/workflow/helpers.py @@ -10,6 +10,7 @@ # import re +import shlex from typing import List, Optional, Tuple from ...error import CommandError @@ -24,6 +25,41 @@ _RESPONSE_CODE_RE = re.compile(r'\s*[Rr]esponse\s+code:\s*\S+\s*$') +def fix_dash_uid_args(parser, args): + """Insert '--' before a base64url UID that starts with '-' so argparse + treats it as a positional value instead of an unknown flag.""" + if not args or '--' in args: + return args + tokens = shlex.split(args) + known_opts = set() + consumes_value = set() + for action in parser._actions: + for opt in action.option_strings: + known_opts.add(opt) + if action.nargs != 0: + consumes_value.add(opt) + + result = [] + skip_next = False + for token in tokens: + if skip_next: + result.append(token) + skip_next = False + continue + if token in known_opts: + result.append(token) + if token in consumes_value: + skip_next = True + continue + if token.startswith('-'): + result.append('--') + result.append(token) + + if len(result) != len(tokens): + return ' '.join(shlex.quote(t) for t in result) + return args + + def sanitize_router_error(error: Exception) -> str: msg = str(error) msg = _RESPONSE_CODE_RE.sub('', msg) diff --git a/keepercommander/commands/workflow/requester_commands.py b/keepercommander/commands/workflow/requester_commands.py index 5cc9e35ba..c78c0e208 100644 --- a/keepercommander/commands/workflow/requester_commands.py +++ b/keepercommander/commands/workflow/requester_commands.py @@ -27,6 +27,7 @@ is_workflow_exempt, print_exempt_message, submit_access_request, + fix_dash_uid_args, ) @@ -186,6 +187,10 @@ class WorkflowStartCommand(Command): def get_parser(self): return WorkflowStartCommand.parser + def execute_args(self, params, args, **kwargs): + args = fix_dash_uid_args(self.get_parser(), args) + return super().execute_args(params, args, **kwargs) + def execute(self, params: KeeperParams, **kwargs): uid = kwargs.get('uid') record_uid, record = RecordResolver.resolve(params, uid, allow_missing=True) @@ -239,6 +244,10 @@ class WorkflowEndCommand(Command): def get_parser(self): return WorkflowEndCommand.parser + def execute_args(self, params, args, **kwargs): + args = fix_dash_uid_args(self.get_parser(), args) + return super().execute_args(params, args, **kwargs) + def execute(self, params: KeeperParams, **kwargs): if kwargs.get('force'): return self._force_checkin(params, **kwargs) diff --git a/keepercommander/commands/workflow/state_commands.py b/keepercommander/commands/workflow/state_commands.py index 459007da7..492d36a75 100644 --- a/keepercommander/commands/workflow/state_commands.py +++ b/keepercommander/commands/workflow/state_commands.py @@ -203,7 +203,7 @@ def _print_table(params, response): a.user if a.user else RecordResolver.resolve_user(params, a.userId) for a in wf.status.approvedBy ] - approved_by = ', '.join(approved_names) + approved_by = '\n'.join(approved_names) rows.append([stage, record_name, record_uid, flow_uid, checked_out_by, approved_by, started, expires]) headers = ['Stage', 'Record Name', 'Record UID', 'Flow UID', 'Checked Out By', 'Approved By', 'Started', 'Expires'] From db77fc4fbc9311653d95f8a0490222eee8783788 Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Wed, 6 May 2026 17:18:45 +0530 Subject: [PATCH 5/7] Add record type validation for workflows --- .../commands/workflow/config_commands.py | 1 + keepercommander/commands/workflow/helpers.py | 26 ++++++++++++++++++- .../commands/workflow/requester_commands.py | 1 + 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/keepercommander/commands/workflow/config_commands.py b/keepercommander/commands/workflow/config_commands.py index 360c41f6c..cad412a52 100644 --- a/keepercommander/commands/workflow/config_commands.py +++ b/keepercommander/commands/workflow/config_commands.py @@ -95,6 +95,7 @@ def get_parser(self): def execute(self, params: KeeperParams, **kwargs): record_uid, record = RecordResolver.resolve(params, kwargs.get('record')) + RecordResolver.validate_workflow_record_type(record) record_uid_bytes = utils.base64_url_decode(record_uid) # Pre-check: surface the "already exists" condition with a clear, diff --git a/keepercommander/commands/workflow/helpers.py b/keepercommander/commands/workflow/helpers.py index 6f2c586d9..7546e5d7a 100644 --- a/keepercommander/commands/workflow/helpers.py +++ b/keepercommander/commands/workflow/helpers.py @@ -363,6 +363,8 @@ def prompt_for_reason_ticket(needs_reason: bool, needs_ticket: bool) -> Tuple[Op class RecordResolver: + WORKFLOW_RECORD_TYPES = {'pamMachine', 'pamDirectory', 'pamDatabase', 'pamRemoteBrowser'} + @staticmethod def resolve(params, record_input, allow_missing=False): if record_input in params.record_cache: @@ -375,6 +377,19 @@ def resolve(params, record_input, allow_missing=False): return None, None raise CommandError('', f'Record "{record_input}" not found') + @staticmethod + def validate_workflow_record_type(record): + """Raise CommandError if the record type doesn't support workflows.""" + if not isinstance(record, vault.TypedRecord): + raise CommandError('', 'Workflows are only supported on PAM records') + if record.record_type not in RecordResolver.WORKFLOW_RECORD_TYPES: + supported = ', '.join(sorted(RecordResolver.WORKFLOW_RECORD_TYPES)) + raise CommandError( + '', + f'Record "{record.title}" is of type "{record.record_type}" which does not support workflows.\n' + f'Supported record types: {supported}' + ) + @staticmethod def get_uid_bytes(params: KeeperParams, record_uid: str) -> bytes: uid_bytes = utils.base64_url_decode(record_uid) @@ -498,10 +513,19 @@ class WorkflowFormatter: workflow_pb2.SUNDAY: 'Sunday', } + BLOCKING_CONDITIONS = {workflow_pb2.AC_TIME, workflow_pb2.AC_APPROVAL} + @staticmethod def format_stage(stage: int, status=None) -> str: if stage == workflow_pb2.WS_READY_TO_START and status is not None: - if not status.startedOn and not status.conditions: + if status.conditions: + has_blocking = any(c in WorkflowFormatter.BLOCKING_CONDITIONS for c in status.conditions) + if has_blocking: + return 'Waiting' + return 'Ready to Start' + if status.approvedBy and not status.startedOn: + return 'Ready to Start' + if not status.startedOn and not status.approvedBy: return 'Needs Action' return WorkflowFormatter.STAGE_MAP.get(stage, f'Unknown ({stage})') diff --git a/keepercommander/commands/workflow/requester_commands.py b/keepercommander/commands/workflow/requester_commands.py index c78c0e208..d493fd70f 100644 --- a/keepercommander/commands/workflow/requester_commands.py +++ b/keepercommander/commands/workflow/requester_commands.py @@ -67,6 +67,7 @@ def execute(self, params: KeeperParams, **kwargs): @staticmethod def _request(params, **kwargs): record_uid, record = RecordResolver.resolve(params, kwargs.get('record')) + RecordResolver.validate_workflow_record_type(record) if is_workflow_exempt(params, record_uid): print_exempt_message(kwargs.get('format', 'table')) return From 2b48148db36e0806cfd39b4aca00cbfe844e22ba Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Wed, 6 May 2026 17:47:10 +0530 Subject: [PATCH 6/7] Add better error handling --- keepercommander/commands/workflow/approver_commands.py | 9 ++++++++- keepercommander/commands/workflow/config_commands.py | 6 ++++++ keepercommander/commands/workflow/helpers.py | 5 ++++- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/keepercommander/commands/workflow/approver_commands.py b/keepercommander/commands/workflow/approver_commands.py index 2b04a6e5f..3b3c564fe 100644 --- a/keepercommander/commands/workflow/approver_commands.py +++ b/keepercommander/commands/workflow/approver_commands.py @@ -244,7 +244,14 @@ def execute(self, params: KeeperParams, **kwargs): if reason: reason_bytes = reason.encode('utf-8') encrypted = self._encrypt_denial_reason(params, flow_uid_bytes, reason_bytes) - denial.denialReason = encrypted if encrypted else reason_bytes + if encrypted: + denial.denialReason = encrypted + else: + logging.warning( + 'Could not encrypt denial reason for the requester — reason will not be attached. ' + 'The denial itself will still be sent.' + ) + reason = '' try: _post_request_to_router(params, 'approve_or_deny_workflow_access', rq_proto=denial) diff --git a/keepercommander/commands/workflow/config_commands.py b/keepercommander/commands/workflow/config_commands.py index cad412a52..6bed4e7dc 100644 --- a/keepercommander/commands/workflow/config_commands.py +++ b/keepercommander/commands/workflow/config_commands.py @@ -453,6 +453,8 @@ def execute(self, params: KeeperParams, **kwargs): print(f"Record: {record.title} ({record_uid})") print() + except CommandError: + raise except Exception as e: raise CommandError('', f'Failed to update workflow: {sanitize_router_error(e)}') @@ -579,6 +581,8 @@ def execute(self, params: KeeperParams, **kwargs): print(f"Type: Escalation approver{esc_info}") print() + except CommandError: + raise except Exception as e: raise CommandError('', f'Failed to add approvers: {sanitize_router_error(e)}') @@ -639,5 +643,7 @@ def execute(self, params: KeeperParams, **kwargs): print(f"Removed {total} approver(s)") print() + except CommandError: + raise except Exception as e: raise CommandError('', f'Failed to remove approvers: {sanitize_router_error(e)}') diff --git a/keepercommander/commands/workflow/helpers.py b/keepercommander/commands/workflow/helpers.py index 7546e5d7a..2e269d917 100644 --- a/keepercommander/commands/workflow/helpers.py +++ b/keepercommander/commands/workflow/helpers.py @@ -30,7 +30,10 @@ def fix_dash_uid_args(parser, args): treats it as a positional value instead of an unknown flag.""" if not args or '--' in args: return args - tokens = shlex.split(args) + try: + tokens = shlex.split(args) + except ValueError: + return args known_opts = set() consumes_value = set() for action in parser._actions: From ca66c3593629bcaca29bf54abc79a0590878690b Mon Sep 17 00:00:00 2001 From: amangalampalli-ks Date: Fri, 8 May 2026 11:13:42 +0530 Subject: [PATCH 7/7] Update for review comments and tzlocal library for windows --- .../commands/workflow/approver_commands.py | 14 +- .../commands/workflow/config_commands.py | 34 +--- keepercommander/commands/workflow/helpers.py | 191 ++++++------------ keepercommander/commands/workflow/mfa.py | 76 +------ .../commands/workflow/requester_commands.py | 14 +- requirements.txt | 1 + setup.cfg | 1 + 7 files changed, 83 insertions(+), 248 deletions(-) diff --git a/keepercommander/commands/workflow/approver_commands.py b/keepercommander/commands/workflow/approver_commands.py index 3b3c564fe..644bc2296 100644 --- a/keepercommander/commands/workflow/approver_commands.py +++ b/keepercommander/commands/workflow/approver_commands.py @@ -22,7 +22,7 @@ from ...proto import workflow_pb2 from ... import api, crypto, utils -from .helpers import RecordResolver, WorkflowFormatter, sanitize_router_error, fix_dash_uid_args +from .helpers import RecordResolver, WorkflowFormatter, sanitize_router_error, DashUidArgsMixin class WorkflowGetApprovalRequestsCommand(Command): @@ -171,7 +171,7 @@ def _print_table(params, workflows): print() -class WorkflowApproveCommand(Command): +class WorkflowApproveCommand(DashUidArgsMixin, Command): parser = argparse.ArgumentParser( prog='pam workflow approve', description='Approve a workflow access request', @@ -183,10 +183,6 @@ class WorkflowApproveCommand(Command): def get_parser(self): return WorkflowApproveCommand.parser - def execute_args(self, params, args, **kwargs): - args = fix_dash_uid_args(self.get_parser(), args) - return super().execute_args(params, args, **kwargs) - def execute(self, params: KeeperParams, **kwargs): flow_uid = kwargs.get('flow_uid') try: @@ -213,7 +209,7 @@ def execute(self, params: KeeperParams, **kwargs): raise CommandError('', f'Failed to approve request: {sanitize_router_error(e)}') -class WorkflowDenyCommand(Command): +class WorkflowDenyCommand(DashUidArgsMixin, Command): parser = argparse.ArgumentParser( prog='pam workflow deny', description='Deny a workflow access request', @@ -226,10 +222,6 @@ class WorkflowDenyCommand(Command): def get_parser(self): return WorkflowDenyCommand.parser - def execute_args(self, params, args, **kwargs): - args = fix_dash_uid_args(self.get_parser(), args) - return super().execute_args(params, args, **kwargs) - def execute(self, params: KeeperParams, **kwargs): flow_uid = kwargs.get('flow_uid') reason = kwargs.get('reason') or '' diff --git a/keepercommander/commands/workflow/config_commands.py b/keepercommander/commands/workflow/config_commands.py index 6bed4e7dc..1fe17d59a 100644 --- a/keepercommander/commands/workflow/config_commands.py +++ b/keepercommander/commands/workflow/config_commands.py @@ -28,13 +28,8 @@ def _add_approvers_to_workflow(params, record_uid, record_name, users=None, teams=None, is_escalation=False, escalation_after_ms=0): - """Send add_workflow_approvers for the given users / teams. Shared by - `pam workflow create` (when --approver flags are supplied) and - `pam workflow add-approver` so both go through one code path. - - Caller is responsible for de-duplicating the user / team lists. Raises - on transport error; caller decides how to surface. - """ + """Send add_workflow_approvers for the given users / teams. Shared by create + add-approver. + Caller must de-duplicate. Raises on transport error.""" record_uid_bytes = utils.base64_url_decode(record_uid) config = workflow_pb2.WorkflowConfig() config.parameters.resource.CopyFrom( @@ -98,10 +93,7 @@ def execute(self, params: KeeperParams, **kwargs): RecordResolver.validate_workflow_record_type(record) record_uid_bytes = utils.base64_url_decode(record_uid) - # Pre-check: surface the "already exists" condition with a clear, - # actionable message instead of letting the user discover it via the - # raw server error from create_workflow_config. Server-side error - # path is still the authoritative gate; this is just nicer UX. + # Pre-check for nicer UX; server is still authoritative on conflicts. try: existing = _post_request_to_router( params, 'read_workflow_config', @@ -125,7 +117,6 @@ def execute(self, params: KeeperParams, **kwargs): if approvals < 0: raise CommandError('', 'Approvals needed must be 0 or greater') - # Normalize and de-duplicate the approver list (preserves first-seen order). approvers = list(dict.fromkeys( a.strip() for a in (kwargs.get('approver') or []) if a and a.strip() )) @@ -162,10 +153,6 @@ def execute(self, params: KeeperParams, **kwargs): try: _post_request_to_router(params, 'create_workflow_config', rq_proto=parameters) - # Step 2: send the explicit approver list (if any). Mirrors web vault - # which issues create_workflow_config + add_workflow_approvers as two - # separate calls (save-workflow-settings.ts:78-99). No silent - # auto-add of the creator / record-owner. approvers_added = [] if approvers: try: @@ -328,9 +315,6 @@ def _print_table(params, response, record_uid): print(f" Days: {', '.join(day_names)}") if at.timeRanges: for tr in at.timeRanges: - # startTime / endTime are HHMM (hours*100 + minutes); see - # WorkflowFormatter._parse_time_to_hhmm and the canonical - # ka-libs/workflow/.../WfConfigCRUD.kt::validateHHMM. start_h, start_m = divmod(tr.startTime, 100) end_h, end_m = divmod(tr.endTime, 100) print(f" Time: {start_h:02d}:{start_m:02d} - {end_h:02d}:{end_m:02d}") @@ -453,7 +437,7 @@ def execute(self, params: KeeperParams, **kwargs): print(f"Record: {record.title} ({record_uid})") print() - except CommandError: + except (CommandError, KeyboardInterrupt, SystemExit): raise except Exception as e: raise CommandError('', f'Failed to update workflow: {sanitize_router_error(e)}') @@ -476,10 +460,7 @@ def execute(self, params: KeeperParams, **kwargs): record_uid_bytes = utils.base64_url_decode(record_uid) ref = ProtobufRefBuilder.record_ref(record_uid_bytes, record.title) - # Pre-check: server-side delete_workflow_config is idempotent and - # returns success even when no config exists, so without this check - # repeat calls all print "deleted successfully" — confusing in - # interactive use. Verify there's actually something to delete first. + # Server-side delete is idempotent; verify config exists for accurate user feedback. try: existing = _post_request_to_router( params, 'read_workflow_config', @@ -530,7 +511,6 @@ def get_parser(self): return WorkflowAddApproversCommand.parser def execute(self, params: KeeperParams, **kwargs): - # De-duplicate user / team lists (preserve first-seen order). users = list(dict.fromkeys( u.strip() for u in (kwargs.get('user') or []) if u and u.strip() )) @@ -581,7 +561,7 @@ def execute(self, params: KeeperParams, **kwargs): print(f"Type: Escalation approver{esc_info}") print() - except CommandError: + except (CommandError, KeyboardInterrupt, SystemExit): raise except Exception as e: raise CommandError('', f'Failed to add approvers: {sanitize_router_error(e)}') @@ -643,7 +623,7 @@ def execute(self, params: KeeperParams, **kwargs): print(f"Removed {total} approver(s)") print() - except CommandError: + except (CommandError, KeyboardInterrupt, SystemExit): raise except Exception as e: raise CommandError('', f'Failed to remove approvers: {sanitize_router_error(e)}') diff --git a/keepercommander/commands/workflow/helpers.py b/keepercommander/commands/workflow/helpers.py index 2e269d917..7dede3c8d 100644 --- a/keepercommander/commands/workflow/helpers.py +++ b/keepercommander/commands/workflow/helpers.py @@ -25,15 +25,25 @@ _RESPONSE_CODE_RE = re.compile(r'\s*[Rr]esponse\s+code:\s*\S+\s*$') +class DashUidArgsMixin: + """Mixin for commands whose positional flow-UID arg may start with '-' (base64url).""" + + def execute_args(self, params, args, **kwargs): + args = fix_dash_uid_args(self.get_parser(), args) + return super().execute_args(params, args, **kwargs) + + def fix_dash_uid_args(parser, args): - """Insert '--' before a base64url UID that starts with '-' so argparse - treats it as a positional value instead of an unknown flag.""" - if not args or '--' in args: + """Insert '--' before a base64url UID starting with '-' so argparse treats it as positional.""" + if not args: return args try: tokens = shlex.split(args) except ValueError: return args + if '--' in tokens: + return args + known_opts = set() consumes_value = set() for action in parser._actions: @@ -49,9 +59,10 @@ def fix_dash_uid_args(parser, args): result.append(token) skip_next = False continue - if token in known_opts: + opt_name = token.split('=', 1)[0] if token.startswith('--') and '=' in token else token + if opt_name in known_opts: result.append(token) - if token in consumes_value: + if opt_name in consumes_value and token == opt_name: skip_next = True continue if token.startswith('-'): @@ -72,7 +83,6 @@ def sanitize_router_error(error: Exception) -> str: def print_exempt_message(fmt='table'): - """Print the standard exemption message in the appropriate format.""" import json as _json from ...display import bcolors as _bc if fmt == 'json': @@ -83,7 +93,6 @@ def print_exempt_message(fmt='table'): def is_record_owner(params, record_uid): - """Check if the current user is the owner of the given record.""" if record_uid in getattr(params, 'record_owner_cache', {}): owner_info = params.record_owner_cache[record_uid] if getattr(owner_info, 'owner', False): @@ -92,8 +101,7 @@ def is_record_owner(params, record_uid): def is_on_approver_list(params, config): - """Check if the current user appears on the workflow config's approver list, - either directly by email or via team membership.""" + """Check if current user is on the approver list (by email or team membership).""" if not config or not config.approvers: return False @@ -111,13 +119,8 @@ def is_on_approver_list(params, config): def is_workflow_exempt(params, record_uid, config=None): - """Exempt users: record owner OR on the approver list. - - If *config* is already available (e.g. from a prior read_workflow_config - call) pass it to avoid an extra API round-trip. When *config* is None the - function reads it from the router; a transport failure is treated as - non-exempt (fail closed). - """ + """Exempt = record owner OR on approver list. Pass `config` to skip a round-trip. + Transport failures fail closed (non-exempt).""" if is_record_owner(params, record_uid): return True @@ -132,51 +135,21 @@ def is_workflow_exempt(params, record_uid, config=None): params, 'read_workflow_config', rq_proto=ref, rs_type=workflow_pb2.WorkflowConfig, ) - except Exception: + except Exception as e: + import logging as _logging + _logging.debug( + 'is_workflow_exempt config read failed for %s: %s', record_uid, e, + ) return False return is_on_approver_list(params, config) def is_pam_action_allowed_by_enforcement(params: KeeperParams, enforcement_key: str) -> bool: - """Per-user enterprise enforcement gate: is this user permitted to perform - the action by their enterprise enforcement profile? - - Mirrors web vault `getAllowConnections` / `getAllowPortForwards` - (pam-enforcement-selectors.ts:38-49). The enforcement boolean is already - the rolled-up sum of the user's role permissions — no additional role / - team / ACL / deny-list checks are needed (verified against WV). - - NOTE — license check intentionally NOT included here. Web vault wraps - these enforcement selectors in `mkPam` (pam-enforcement-selectors.ts:33-34) - which also requires `state.accountSummary.license?.isPamEnabled`. Commander - has historically treated license defensively: we let the gateway / server - be the authoritative gate for license-driven failures rather than - pre-flighting it client-side. Same approach is used by the rotation - enforcement check (`_is_rotation_allowed_by_enforcement` in - discoveryrotation.py). The trade-off: an account that has the enforcement - granted but no PAM license will pass this gate and fail later at the - gateway with a more specific error, instead of getting a "not licensed" - refusal up front. If parity becomes important, wrap this with a license - check against `params.account_summary` / `params.license`. - - Decision matrix: - no enforcement context at all (personal / non-enterprise user): - -> allow (gateway is the authoritative gate) - enforcement context present, key explicitly true: - -> allow - enforcement context present, key explicitly false: - -> deny - enforcement context present, key absent: - -> deny (matches WV: `!!enforcements.` evaluates to false - for missing keys; also matches Commander's enforcement parser - behavior, which converts `--enforcement KEY:false` to None and - removes the key, so "absent" is the actual on-the-wire shape - of "explicitly disabled") - - Defensively swallows any unexpected enforcement payload shape and falls - back to allow. - """ + """Per-user enterprise enforcement gate. Mirrors web vault's PAM enforcement selectors. + Non-enterprise users and unexpected payload shapes fall back to allow (gateway gates). + Enterprise users with the key absent are denied (matches WV's `!!enforcements.`). + License is intentionally not checked here — gateway is the authoritative gate for that.""" import logging as _logging try: enforcements = getattr(params, 'enforcements', None) @@ -184,14 +157,10 @@ def is_pam_action_allowed_by_enforcement(params: KeeperParams, enforcement_key: return True booleans = enforcements.get('booleans') or [] if not isinstance(booleans, list) or not booleans: - # Enforcement context but no booleans at all — treat as - # "not yet configured" and allow; gateway will gate. return True for b in booleans: if isinstance(b, dict) and b.get('key') == enforcement_key: return bool(b.get('value')) - # Enterprise user with a populated enforcement set, but the - # specific PAM-grant key is absent. Match WV: deny. return False except Exception as e: _logging.debug('Enforcement check failed for %s: %s', enforcement_key, e) @@ -200,27 +169,9 @@ def is_pam_action_allowed_by_enforcement(params: KeeperParams, enforcement_key: def is_pam_config_action_allowed_for_record(params: KeeperParams, record_uid: str, action_key: str) -> bool: - """Best-effort PAM config gate: is the action permitted by the record's - PAM configuration's allowedSettings (DAG) ? - - Mirrors web vault `getConfigAllowedSettings(recordUid)[key]` (used by - GuacConnectBanner.tsx:37-45 for launch and StartPortForwardButton.tsx:160-163 - for tunnel). The action_key matches the JSON-mapped names returned by - PAMConfigurationListCommand._allowed_settings_dag_to_json: - - 'connections' → launch / connect - 'tunneling' → tunnel / port-forward (DAG: portForwards) - 'rotation' → rotate - - Returns True (allow) on any lookup failure (no PAM context, missing DAG, - not a PAM record, personal account) so non-enterprise contexts aren't - blocked. Returns False only when the flag is explicitly False on the - PAM config DAG. - - Fast path: launch_cache entry holds a recent config_uid for the record; - on cache hit we skip the DAG-leafs round-trip and go straight to the - config-vertex read. - """ + """Best-effort PAM config allowedSettings (DAG) gate. + action_key: 'connections' (launch), 'tunneling' (port-forward), 'rotation'. + Returns True on any lookup failure; False only when explicitly disabled on the config.""" import logging as _logging try: config_uid = None @@ -257,16 +208,7 @@ def is_pam_config_action_allowed_for_record(params: KeeperParams, record_uid: st def is_gateway_online_for_record(params: KeeperParams, record_uid: str) -> Optional[bool]: - """Best-effort check: is the gateway for record_uid currently connected to the router? - - Returns True / False when known, None when undetermined (e.g. the - record has no entry in launch_cache yet, or the router lookup fails). - Callers should treat None as "proceed with normal flow" and only act - on a definitive False. - - Uses launch_cache.get to avoid an expensive PAM_LINK DAG rebuild on the - first launch; subsequent launches resolve the gateway UID instantly. - """ + """Best-effort gateway online check. Returns None when undetermined (treat as 'proceed').""" import logging as _logging try: from ..pam_launch import launch_cache @@ -289,7 +231,6 @@ def is_gateway_online_for_record(params: KeeperParams, record_uid: str) -> Optio def start_workflow_for_record(params: KeeperParams, record_uid: str) -> None: - """Send a start_workflow (check-out) request for the given record.""" from ..pam.router_helper import _post_request_to_router record_uid_bytes = utils.base64_url_decode(record_uid) record = vault.KeeperRecord.load(params, record_uid) @@ -302,11 +243,7 @@ def start_workflow_for_record(params: KeeperParams, record_uid: str) -> None: def submit_access_request(params: KeeperParams, record_uid: str, reason: str = '', ticket: str = '') -> None: - """Send a workflow access request with optional encrypted reason/ticket. - - Encrypts reason/ticket with the record key (AES-GCM-256), matching - web vault request_workflow_access.ts. Caller must hold the record key. - """ + """Send a workflow access request. Reason/ticket are encrypted with the record key.""" from ..pam.router_helper import _post_request_to_router record_uid_bytes = utils.base64_url_decode(record_uid) record = vault.KeeperRecord.load(params, record_uid) @@ -332,12 +269,7 @@ def submit_access_request(params: KeeperParams, record_uid: str, def prompt_for_reason_ticket(needs_reason: bool, needs_ticket: bool) -> Tuple[Optional[str], Optional[str]]: - """Interactively prompt the user for reason and/or ticket using prompt_toolkit. - - Returns (reason, ticket). Either may be None if not requested. Returns - (None, None) if the user cancels (Ctrl+C / EOF) or submits empty input - for a required field. - """ + """Prompt for reason/ticket. Returns (None, None) on cancel or empty required input.""" from prompt_toolkit import prompt as pt_prompt from ...display import bcolors as _bc @@ -382,14 +314,14 @@ def resolve(params, record_input, allow_missing=False): @staticmethod def validate_workflow_record_type(record): - """Raise CommandError if the record type doesn't support workflows.""" if not isinstance(record, vault.TypedRecord): raise CommandError('', 'Workflows are only supported on PAM records') - if record.record_type not in RecordResolver.WORKFLOW_RECORD_TYPES: + record_type = record.record_type or 'unknown' + if record_type not in RecordResolver.WORKFLOW_RECORD_TYPES: supported = ', '.join(sorted(RecordResolver.WORKFLOW_RECORD_TYPES)) raise CommandError( '', - f'Record "{record.title}" is of type "{record.record_type}" which does not support workflows.\n' + f'Record "{record.title}" is of type "{record_type}" which does not support workflows.\n' f'Supported record types: {supported}' ) @@ -607,6 +539,7 @@ def build_temporal_filter(allowed_days_str, time_range_str): @staticmethod def _get_local_iana_timezone(): + """Detect local IANA timezone via TZ env var (override) or tzlocal (cross-platform).""" import os tz = os.environ.get('TZ') @@ -614,39 +547,32 @@ def _get_local_iana_timezone(): return tz try: - link = os.readlink('/etc/localtime') - marker = '/zoneinfo/' - idx = link.find(marker) - if idx != -1: - return link[idx + len(marker):] - except (OSError, ValueError): - pass + from tzlocal import get_localzone_name + except ImportError: + raise CommandError( + '', + 'Timezone detection requires the "tzlocal" package. ' + 'Install it with: pip install tzlocal\n' + 'Or set the TZ environment variable (e.g., TZ=Asia/Kolkata).' + ) try: - with open('/etc/timezone', 'r') as f: - tz = f.read().strip() - if tz and '/' in tz: - return tz - except (OSError, IOError): - pass + zone = get_localzone_name() + if zone: + return zone + except Exception as e: + import logging as _logging + _logging.debug('tzlocal lookup failed: %s', e) - raise CommandError('', 'Could not detect local IANA timezone. ' - 'Set the TZ environment variable (e.g., TZ=Asia/Kolkata)') + raise CommandError( + '', + 'Could not detect local IANA timezone. ' + 'Set the TZ environment variable (e.g., TZ=Asia/Kolkata).' + ) @staticmethod def _parse_time_to_hhmm(time_str): - """Parse 'HH:MM' to the HHMM integer the server stores on - TimeOfDayRange.startTime / .endTime: hours*100 + minutes. - Examples: '00:00' -> 0, '03:00' -> 300, '09:00' -> 900, '17:30' -> 1730. - Valid range: 0..2359 with hours in 0-23 and minutes in 0-59. - - Canonical sources (all agree on HHMM): - - keeperapp-protobuf/workflow.proto:140 - `int32 startTime = 1; // HHMM format` - - ka-libs/workflow/src/main/kotlin/com/keepersecurity/workflow/handlers/WfConfigCRUD.kt::validateHHMM - `val hours = value / 100; val minutes = value % 100` - throws "Invalid : . Expected HHMM integer with HH in 0-23 and MM in 0-59" on bad input. - """ + """Parse 'HH:MM' to HHMM integer (hours*100 + minutes), e.g. '09:00' -> 900, '17:30' -> 1730.""" try: parts = time_str.split(':') h = int(parts[0]) @@ -667,7 +593,6 @@ def format_temporal_filter(at): if at.timeRanges: ranges = [] for tr in at.timeRanges: - # startTime / endTime are HHMM integers (see _parse_time_to_hhmm). sh, sm = divmod(tr.startTime, 100) eh, em = divmod(tr.endTime, 100) ranges.append(f"{sh:02d}:{sm:02d}-{eh:02d}:{em:02d}") diff --git a/keepercommander/commands/workflow/mfa.py b/keepercommander/commands/workflow/mfa.py index ac883de76..7322325ad 100644 --- a/keepercommander/commands/workflow/mfa.py +++ b/keepercommander/commands/workflow/mfa.py @@ -41,7 +41,7 @@ class WorkflowGate(NamedTuple): - """Result of the pre-launch workflow gate, consumed by pam launch / pam tunnel.""" + """Result of the pre-launch workflow gate.""" allowed: bool two_factor_value: Optional[str] = None flow_uid: Optional[bytes] = None @@ -108,10 +108,6 @@ def validate(self, silent_actionable: bool = False) -> dict: ) return dict(self._DEFAULT_RESULT) if workflow is None: - # Carry the workflow config's required-field flags back so the - # orchestrator can inline-prompt + submit the initial access - # request from no_workflow state, matching web vault's "click - # Launch -> reason/ticket dialog" first-time flow. requires_reason = bool(config.parameters and config.parameters.requireReason) requires_ticket = bool(config.parameters and config.parameters.requireTicket) approvals_needed = int(config.parameters.approvalsNeeded) if config.parameters else 0 @@ -193,9 +189,6 @@ def _check_allowed_times(self, config) -> bool: return False if at.timeRanges: - # TimeOfDayRange.startTime / .endTime are HHMM-encoded integers - # (server-validated: HH in 0-23, MM in 0-59). e.g. 03:00 -> 300, - # 17:30 -> 1730. Compare current wall-clock in the same encoding. current_hhmm = now.hour * 100 + now.minute in_range = False for r in at.timeRanges: @@ -347,6 +340,8 @@ def _print_needs_start(self): class WorkflowMfaPrompt: + _NO_2FA_CONFIGURED = object() + def __init__(self, params: KeeperParams): self.params = params @@ -389,8 +384,6 @@ def prompt(self): return self._dispatch(selected.channelType, APIRequest_pb2) - _NO_2FA_CONFIGURED = object() - @staticmethod def _fetch_2fa_list(params, api, APIRequest_pb2): try: @@ -538,12 +531,7 @@ def check_workflow_access(params: KeeperParams, record_uid: str) -> dict: def _poll_until_state_change(validator: 'WorkflowAccessValidator', timeout_seconds: int) -> Optional[dict]: - """Poll the workflow state at _WAIT_POLL_INTERVAL_SECONDS until the - state is no longer 'waiting' or until timeout_seconds elapses. - - Returns the new validator result dict on state change, or None on - timeout / Ctrl+C / transport error. - """ + """Poll until state is no longer 'waiting' or timeout. Returns None on timeout/cancel/error.""" import time as _time deadline = _time.time() + max(timeout_seconds, _WAIT_POLL_INTERVAL_SECONDS) print( @@ -556,8 +544,6 @@ def _poll_until_state_change(validator: 'WorkflowAccessValidator', r = validator.validate(silent_actionable=True) block_reason = r.get('block_reason') if r.get('allowed', True) or block_reason != 'waiting': - # Suppress chatty output on state change; orchestrator will - # handle the next state in its own loop iteration. return r print(f"{bcolors.WARNING}Approval not received within {timeout_seconds}s.{bcolors.ENDC}\n") return None @@ -576,32 +562,8 @@ def check_workflow_for_launch( wait: bool = False, wait_timeout: int = 600, ) -> WorkflowGate: - """Pre-launch workflow gate: validate access, optionally submit a missing - reason/ticket request and check out the record inline, prompt for MFA if - required, and return the active flow's UID and lease expiry (millis since - epoch) so callers can auto check-in and force-disconnect on lease expiry. - - Three actionable validator states are auto-handled: - - * **`'no_workflow'` / `'needs_start'`** — workflow config exists on the - record but no flow process exists yet for the user (first-time access). - If the config requires a reason / ticket, the supplied --reason / - --ticket flags are used or the user is prompted inline; the initial - access request is submitted via `submit_access_request`. Mirrors web - vault's "click Launch -> reason/ticket dialog" on first-time access. - - * **`'needs_action'`** — flow exists in WS_NEEDS_ACTION with - AC_REASON / AC_TICKET pending; same prompt+submit logic but driven off - the conditions list returned by the server. - - * **`'ready_to_start'`** — flow approved but not yet checked out; user is - prompted (or `auto_checkout=True` confirms automatically), then - `start_workflow` is called and the gate reports `started_by_launch=True` - so the caller can release the lease in its cleanup path. - - Optional `wait=True` polls past `'waiting'` until approval or - `wait_timeout` elapses. - """ + """Pre-launch workflow gate: validate, auto-handle no_workflow/needs_action/ready_to_start, + prompt for MFA, and return flow UID + lease expiry. With `wait=True`, polls past 'waiting'.""" validator = WorkflowAccessValidator(params, record_uid) started_by_launch = False handled_needs_action = False @@ -609,8 +571,7 @@ def check_workflow_for_launch( handled_waiting = False handled_no_workflow = False - # Up to 5 transitions: - # no_workflow -> needs_action -> waiting -> ready_to_start -> started. + # no_workflow -> needs_action -> waiting -> ready_to_start -> started (max 5 transitions). for _attempt in range(5): result = validator.validate(silent_actionable=True) if result.get('allowed', True): @@ -619,13 +580,6 @@ def check_workflow_for_launch( if (block_reason in ('no_workflow', 'needs_start') and not handled_no_workflow): - # First-time access on a workflow-protected record: no flow row - # exists for this user yet. Match web vault's "click Launch -> - # reason/ticket dialog" first-time flow by inline-prompting - # for the required fields (driven off the workflow config's - # require* flags carried in the validator result) and submitting - # the initial access request. Re-validate to land in waiting / - # needs_action / ready_to_start / started naturally. handled_no_workflow = True requires_reason = bool(result.get('requires_reason')) requires_ticket = bool(result.get('requires_ticket')) @@ -746,12 +700,7 @@ def check_workflow_for_launch( break continue - # Block reason we don't auto-handle (waiting w/o --wait, - # transport_error, outside_time_window, no_status, or a re-visit of - # an already-handled actionable). Validator paths print their own - # message in non-silent branches; for the silent-suppressed ones - # (needs_action, ready_to_start, waiting, no_workflow, needs_start) - # fall back to the explicit print so the user always sees something. + # Fall-through for unhandled states; print explicit message so user always sees something. if block_reason == 'needs_action': validator._print_needs_action( result.get('pending_conditions') or (), @@ -775,10 +724,7 @@ def check_workflow_for_launch( two_factor_value = None if result.get('require_mfa', False): - # Match web vault: skip the MFA prompt when the gateway is known to - # be offline. The launch will surface its own gateway-offline error - # later. is_gateway_online_for_record returns None on first launch - # (no cache yet) — in that case keep the prompt to be safe. + # Skip MFA prompt when gateway is known offline; launch surfaces its own error. if is_gateway_online_for_record(params, record_uid) is False: logging.debug("Skipping workflow MFA prompt — gateway is offline.") else: @@ -796,8 +742,6 @@ def check_workflow_for_launch( def check_workflow_and_prompt_2fa(params: KeeperParams, record_uid: str): - """Backward-compatible wrapper around check_workflow_for_launch. - Prefer check_workflow_for_launch in new code — it carries flow_uid and - expires_on_ms needed for auto check-in and lease-expiry teardown.""" + """Backward-compatible wrapper. Prefer check_workflow_for_launch (carries flow_uid + expiry).""" gate = check_workflow_for_launch(params, record_uid) return (gate.allowed, gate.two_factor_value) diff --git a/keepercommander/commands/workflow/requester_commands.py b/keepercommander/commands/workflow/requester_commands.py index d493fd70f..decdb83f6 100644 --- a/keepercommander/commands/workflow/requester_commands.py +++ b/keepercommander/commands/workflow/requester_commands.py @@ -27,7 +27,7 @@ is_workflow_exempt, print_exempt_message, submit_access_request, - fix_dash_uid_args, + DashUidArgsMixin, ) @@ -175,7 +175,7 @@ def _cancel(params, **kwargs): raise CommandError('', f'Failed to cancel request: {sanitize_router_error(e)}') -class WorkflowStartCommand(Command): +class WorkflowStartCommand(DashUidArgsMixin, Command): parser = argparse.ArgumentParser( prog='pam workflow start', description='Start a workflow (check-out). ' @@ -188,10 +188,6 @@ class WorkflowStartCommand(Command): def get_parser(self): return WorkflowStartCommand.parser - def execute_args(self, params, args, **kwargs): - args = fix_dash_uid_args(self.get_parser(), args) - return super().execute_args(params, args, **kwargs) - def execute(self, params: KeeperParams, **kwargs): uid = kwargs.get('uid') record_uid, record = RecordResolver.resolve(params, uid, allow_missing=True) @@ -231,7 +227,7 @@ def execute(self, params: KeeperParams, **kwargs): raise CommandError('', f'Failed to start workflow: {sanitize_router_error(e)}') -class WorkflowEndCommand(Command): +class WorkflowEndCommand(DashUidArgsMixin, Command): parser = argparse.ArgumentParser( prog='pam workflow end', description='End a workflow (check-in).', @@ -245,10 +241,6 @@ class WorkflowEndCommand(Command): def get_parser(self): return WorkflowEndCommand.parser - def execute_args(self, params, args, **kwargs): - args = fix_dash_uid_args(self.get_parser(), args) - return super().execute_args(params, args, **kwargs) - def execute(self, params: KeeperParams, **kwargs): if kwargs.get('force'): return self._force_checkin(params, **kwargs) diff --git a/requirements.txt b/requirements.txt index 41983e843..f18fc03e7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -26,5 +26,6 @@ winrt-runtime; sys_platform == "win32" winrt-Windows.Foundation; sys_platform == "win32" winrt-Windows.Security.Credentials.UI; sys_platform == "win32" googleapis-common-protos +tzlocal>=5.0 keeper-mlkem; python_version>='3.11' textual; python_version>='3.9' \ No newline at end of file diff --git a/setup.cfg b/setup.cfg index ac7f6bfb8..d19540ae1 100644 --- a/setup.cfg +++ b/setup.cfg @@ -53,6 +53,7 @@ install_requires = keeper_pam_webrtc_rs>=2.1.6 pydantic>=2.6.4 fpdf2>=2.8.3 + tzlocal>=5.0 cbor2; sys_platform == "darwin" and python_version>='3.10' pyobjc-framework-LocalAuthentication; sys_platform == "darwin" and python_version>='3.10' winrt-runtime; sys_platform == "win32" and python_version>='3.10'