diff --git a/keepercommander/commands/workflow/approver_commands.py b/keepercommander/commands/workflow/approver_commands.py index 342579714..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 +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', @@ -209,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', @@ -236,7 +236,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 28ca9c71f..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( @@ -83,8 +78,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. ' @@ -97,12 +90,10 @@ 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, - # 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', @@ -126,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() )) @@ -155,7 +145,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) @@ -163,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: @@ -329,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}") @@ -386,8 +369,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 +415,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) @@ -456,6 +437,8 @@ def execute(self, params: KeeperParams, **kwargs): print(f"Record: {record.title} ({record_uid})") print() + except (CommandError, KeyboardInterrupt, SystemExit): + raise except Exception as e: raise CommandError('', f'Failed to update workflow: {sanitize_router_error(e)}') @@ -477,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', @@ -531,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() )) @@ -582,6 +561,8 @@ def execute(self, params: KeeperParams, **kwargs): print(f"Type: Escalation approver{esc_info}") print() + except (CommandError, KeyboardInterrupt, SystemExit): + raise except Exception as e: raise CommandError('', f'Failed to add approvers: {sanitize_router_error(e)}') @@ -642,5 +623,7 @@ def execute(self, params: KeeperParams, **kwargs): print(f"Removed {total} approver(s)") print() + 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 21a3ac4dd..7dede3c8d 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,55 @@ _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 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: + 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 + opt_name = token.split('=', 1)[0] if token.startswith('--') and '=' in token else token + if opt_name in known_opts: + result.append(token) + if opt_name in consumes_value and token == opt_name: + 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) @@ -32,90 +82,74 @@ 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 from ...display import bcolors as _bc 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): 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 current user is on the approver list (by email or 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 = 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 + + 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 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) @@ -123,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) @@ -139,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 @@ -196,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 @@ -228,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) @@ -241,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) @@ -271,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 @@ -305,6 +298,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: @@ -317,6 +312,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): + if not isinstance(record, vault.TypedRecord): + raise CommandError('', 'Workflows are only supported on PAM records') + 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_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) @@ -440,10 +448,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})') @@ -490,8 +507,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,25 +533,46 @@ 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(): + """Detect local IANA timezone via TZ env var (override) or tzlocal (cross-platform).""" + import os + + tz = os.environ.get('TZ') + if tz and '/' in tz: + return tz + + try: + 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: + 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).' + ) + @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]) @@ -555,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 d9a977eac..7322325ad 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, @@ -40,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 @@ -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): @@ -119,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 @@ -204,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: @@ -323,8 +305,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() @@ -359,6 +340,8 @@ def _print_needs_start(self): class WorkflowMfaPrompt: + _NO_2FA_CONFIGURED = object() + def __init__(self, params: KeeperParams): self.params = params @@ -367,6 +350,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() @@ -405,12 +395,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 @@ -546,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( @@ -564,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 @@ -584,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 @@ -617,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): @@ -627,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')) @@ -754,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 (), @@ -783,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: @@ -804,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/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, diff --git a/keepercommander/commands/workflow/requester_commands.py b/keepercommander/commands/workflow/requester_commands.py index 5cc9e35ba..decdb83f6 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, + DashUidArgsMixin, ) @@ -66,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 @@ -173,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). ' @@ -225,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).', diff --git a/keepercommander/commands/workflow/state_commands.py b/keepercommander/commands/workflow/state_commands.py index 751b7c8c4..492d36a75 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( @@ -204,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) @@ -214,10 +203,10 @@ 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) - rows.append([stage, record_name, record_uid, flow_uid, checked_out_by, approved_by, started, expires, conditions]) + 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', 'Conditions'] + headers = ['Stage', 'Record Name', 'Record UID', 'Flow UID', 'Checked Out By', 'Approved By', 'Started', 'Expires'] print() dump_report_data(rows, headers=headers) print() 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'