diff --git a/keepercommander/service/commands/integrations/approvals_setup.py b/keepercommander/service/commands/integrations/approvals_setup.py index 50e4d0fc3..9aadc24f5 100644 --- a/keepercommander/service/commands/integrations/approvals_setup.py +++ b/keepercommander/service/commands/integrations/approvals_setup.py @@ -46,6 +46,8 @@ class ApprovalsChannelProfile: channel_prompt: str validate_channel: Callable[[str], bool] channel_error: str + channel_field_name: str = 'approvals_channel_id' + team_channel_key: str = 'channel_id' SLACK_APPROVALS_PROFILE = ApprovalsChannelProfile( @@ -59,6 +61,8 @@ class ApprovalsChannelProfile: channel_prompt='Channel ID (starts with C):', validate_channel=lambda c: bool(c and c.startswith('C')), channel_error="Invalid Approvals Channel ID (must start with 'C')", + channel_field_name='approvals_channel_id', + team_channel_key='channel_id', ) @@ -317,14 +321,17 @@ def _collect_team_boundaries( return updated -def approvals_config_to_record_fields(config: ApprovalsConfig) -> List: +def approvals_config_to_record_fields(config: ApprovalsConfig, profile: ApprovalsChannelProfile = None) -> List: + if profile is None: + profile = SLACK_APPROVALS_PROFILE + teams_json = '' if config.multi_channel_enabled: teams_json = json.dumps([ { 'team_uid': team.team_uid, 'name': team.name, - 'channel_id': team.channel_id, + profile.team_channel_key: team.channel_id, 'folder_uids': team.folder_uids, 'record_uids': team.record_uids, } @@ -340,7 +347,7 @@ def approvals_config_to_record_fields(config: ApprovalsConfig) -> List: vault.TypedField.new_field( 'text', config.single_channel_id, - FIELD_APPROVALS_CHANNEL_ID, + profile.channel_field_name, ), vault.TypedField.new_field('multiline', teams_json, FIELD_APPROVALS_TEAMS), ] diff --git a/keepercommander/service/commands/integrations/approvals_sync.py b/keepercommander/service/commands/integrations/approvals_sync.py index e82045365..3028c2d9d 100644 --- a/keepercommander/service/commands/integrations/approvals_sync.py +++ b/keepercommander/service/commands/integrations/approvals_sync.py @@ -74,10 +74,15 @@ def custom_fields_by_label(record: vault.KeeperRecord) -> Dict[str, str]: def parse_approvals_from_record( record: vault.KeeperRecord, command_name: str = '', + profile: 'ApprovalsChannelProfile' = None, ) -> ApprovalsConfig: + from .approvals_setup import SLACK_APPROVALS_PROFILE + if profile is None: + profile = SLACK_APPROVALS_PROFILE + fields = custom_fields_by_label(record) multi_channel = fields.get(FIELD_MULTI_CHANNEL_ENABLED, 'false').strip().lower() == 'true' - single_channel_id = fields.get(FIELD_APPROVALS_CHANNEL_ID, '').strip() + single_channel_id = fields.get(profile.channel_field_name, '').strip() teams_json = fields.get(FIELD_APPROVALS_TEAMS, '').strip() teams: List[ApproverTeam] = [] @@ -94,7 +99,7 @@ def parse_approvals_from_record( if not isinstance(item, dict): continue team_uid = str(item.get('team_uid', '')).strip() - channel_id = str(item.get('channel_id', '')).strip() + channel_id = str(item.get(profile.team_channel_key, '')).strip() if not team_uid or not channel_id or team_uid in seen_team_uids: continue seen_team_uids.add(team_uid) @@ -117,14 +122,15 @@ def parse_approvals_from_record( ) -def merge_approvals_custom_fields(existing_custom: List, config: ApprovalsConfig) -> List: - preserved = [f for f in (existing_custom or []) if f.label not in APPROVALS_FIELD_LABELS] - return preserved + approvals_config_to_record_fields(config) +def merge_approvals_custom_fields(existing_custom: List, config: ApprovalsConfig, profile: 'ApprovalsChannelProfile' = None) -> List: + preserved = [f for f in (existing_custom or []) if f.label not in APPROVALS_FIELD_LABELS and (profile is None or f.label != profile.channel_field_name)] + return preserved + approvals_config_to_record_fields(config, profile) def analyze_approvals_drift( params: 'KeeperParams', config: ApprovalsConfig, + profile: 'ApprovalsChannelProfile' = None, ) -> Tuple[ApprovalsConfig, ApprovalsDriftReport]: by_uid, _ = build_team_lookup(params) known_folders = build_shared_folder_uids(params) @@ -194,6 +200,7 @@ def sync_approvals_config( params: 'KeeperParams', config: ApprovalsConfig, command_name: str = '', + profile: 'ApprovalsChannelProfile' = None, ) -> Tuple[ApprovalsConfig, ApprovalsDriftReport]: """Remove stale vault references and refresh team names. Non-interactive.""" if not config.multi_channel_enabled: @@ -203,7 +210,7 @@ def sync_approvals_config( 'Re-run setup or enable multi-channel approvers first.', ) - cleaned, report = analyze_approvals_drift(params, config) + cleaned, report = analyze_approvals_drift(params, config, profile) print_approvals_drift_report(report) if not cleaned.teams and config.teams: @@ -222,6 +229,7 @@ def run_approvals_sync_down( update_record: Callable[[str, ApprovalsConfig], None], command_name: str = '', sync_vault: bool = True, + profile: 'ApprovalsChannelProfile' = None, ) -> ApprovalsConfig: if sync_vault: params.sync_data = True @@ -239,8 +247,8 @@ def run_approvals_sync_down( f'(missing "{marker_field}" field)', ) - config = parse_approvals_from_record(record, command_name=command_name) - updated, report = sync_approvals_config(params, config, command_name=command_name) + config = parse_approvals_from_record(record, command_name=command_name, profile=profile) + updated, report = sync_approvals_config(params, config, command_name=command_name, profile=profile) if report.has_changes: update_record(record_uid, updated) diff --git a/keepercommander/service/commands/integrations/gchat_app_setup.py b/keepercommander/service/commands/integrations/gchat_app_setup.py index cdc1f00d4..20a33d542 100644 --- a/keepercommander/service/commands/integrations/gchat_app_setup.py +++ b/keepercommander/service/commands/integrations/gchat_app_setup.py @@ -19,7 +19,13 @@ from .... import vault from ....display import bcolors +from ....error import CommandError from ...docker import GChatConfig, GChatConstants +from .approvals_setup import ( + ApprovalsChannelProfile, + approvals_config_to_record_fields, + print_approvals_config, +) from .integration_setup_base import IntegrationSetupCommand # Short Pub/Sub IDs, or full resource names: @@ -33,6 +39,26 @@ r'^projects/([^/]+)/topics/([A-Za-z][\w.-]{2,})$' ) +def _validate_gchat_space_id(space_id: str) -> bool: + prefix = GChatConstants.SPACE_ID_PREFIX + return bool(space_id and space_id.startswith(prefix) and len(space_id) > len(prefix)) + + +GCHAT_APPROVALS_PROFILE = ApprovalsChannelProfile( + channel_header='APPROVALS_SPACE_ID', + single_channel_description='Google Chat space ID for approval notifications', + channel_description='Google Chat space where approval requests for this approver team are sent', + default_channel_description=( + 'Default space for EPM privilege requests, SSO Cloud device approvals, ' + 'and approval requests from users not assigned to an approver team' + ), + channel_prompt='Space ID (starts with spaces/):', + validate_channel=_validate_gchat_space_id, + channel_error="Invalid Approvals Space ID (must start with 'spaces/' and include a space name)", + channel_field_name='chat_approvals_space_id', + team_channel_key='space_id', +) + class GChatAppSetupCommand(IntegrationSetupCommand): """Google Chat App integration setup.""" @@ -52,6 +78,9 @@ def get_default_record_name(self) -> str: def get_integration_config_marker_field(self) -> str: return GChatConstants.FIELD_SERVICE_ACCOUNT_JSON + def get_approvals_profile(self): + return GCHAT_APPROVALS_PROFILE + # ── Google Chat-specific configuration ──────────────────────── def collect_integration_config(self, params): @@ -82,14 +111,6 @@ def collect_integration_config(self, params): google_project_id, ) - print(f"\n{bcolors.BOLD}CHAT_APPROVALS_SPACE_ID:{bcolors.ENDC}") - print(f" Google Chat space where approval cards are posted") - chat_approvals_space_id = self._prompt_with_validation( - "Space ID (starts with spaces/):", - self._is_valid_space_id, - "Invalid Approvals Space ID (must start with 'spaces/' and include a space name)" - ) - print(f"\n{bcolors.BOLD}CHAT COMMAND IDs:{bcolors.ENDC}") print(f" Slash command IDs configured for the Google Chat app") chat_command_request_record_id = self._prompt_command_id( @@ -109,6 +130,14 @@ def collect_integration_config(self, params): GChatConstants.DEFAULT_COMMAND_CREATE_SECRET_ID, ) + profile = self.get_approvals_profile() + if profile is None: + raise CommandError( + self.get_command_name(), + 'Internal error: Google Chat approvals profile not configured' + ) + approvals = self._collect_approvals_config(params, profile) + pedm_enabled, pedm_interval = self._collect_pedm_config() da_enabled, da_interval = self._collect_device_approval_config() @@ -119,7 +148,7 @@ def collect_integration_config(self, params): google_project_id=google_project_id, google_subscription_id=google_subscription_id, google_topic_id=google_topic_id, - chat_approvals_space_id=chat_approvals_space_id, + approvals=approvals, chat_command_request_record_id=chat_command_request_record_id, chat_command_request_folder_id=chat_command_request_folder_id, chat_command_external_share_id=chat_command_external_share_id, @@ -146,11 +175,6 @@ def build_record_custom_fields(self, config): vault.TypedField.new_field( 'text', config.google_topic_id, GChatConstants.FIELD_TOPIC_ID ), - vault.TypedField.new_field( - 'text', - config.chat_approvals_space_id, - GChatConstants.FIELD_APPROVALS_SPACE_ID, - ), vault.TypedField.new_field( 'text', config.chat_command_request_record_id, @@ -171,6 +195,7 @@ def build_record_custom_fields(self, config): config.chat_command_create_secret_id, GChatConstants.FIELD_COMMAND_CREATE_SECRET_ID, ), + *approvals_config_to_record_fields(config.approvals, self.get_approvals_profile()), vault.TypedField.new_field( 'text', 'true' if config.pedm_enabled else 'false', @@ -202,10 +227,6 @@ def print_integration_specific_resources(self, config): f" • Pub/Sub Subscription: " f"{bcolors.OKBLUE}{config.google_subscription_id}{bcolors.ENDC}" ) - print( - f" • Approvals Space: " - f"{bcolors.OKBLUE}{config.chat_approvals_space_id}{bcolors.ENDC}" - ) print( f" • /keeper-request-record ID: " f"{bcolors.OKBLUE}{config.chat_command_request_record_id}{bcolors.ENDC}" @@ -222,6 +243,7 @@ def print_integration_specific_resources(self, config): f" • /keeper-create-secret ID: " f"{bcolors.OKBLUE}{config.chat_command_create_secret_id}{bcolors.ENDC}" ) + print_approvals_config(config.approvals) def print_integration_commands(self): print(f"\n{bcolors.BOLD}Google Chat Commands Available:{bcolors.ENDC}") @@ -315,7 +337,7 @@ def _load_service_account_json(cls, path: str) -> tuple[dict | None, str | None] return cls._validate_service_account_dict(data) @staticmethod - def _validate_service_account_dict(data: any) -> tuple[dict | None, str | None]: + def _validate_service_account_dict(data: object) -> tuple[dict | None, str | None]: if not isinstance(data, dict): return None, 'Service account JSON must be a JSON object' @@ -389,7 +411,3 @@ def _normalize_topic_id( google_project_id, ) - @staticmethod - def _is_valid_space_id(value: str) -> bool: - prefix = GChatConstants.SPACE_ID_PREFIX - return bool(value and value.startswith(prefix) and len(value) > len(prefix)) \ No newline at end of file diff --git a/keepercommander/service/commands/integrations/integration_setup_base.py b/keepercommander/service/commands/integrations/integration_setup_base.py index aea4cb932..3d26a5169 100644 --- a/keepercommander/service/commands/integrations/integration_setup_base.py +++ b/keepercommander/service/commands/integrations/integration_setup_base.py @@ -383,7 +383,8 @@ def _patch_record_approvals(self, params, record_uid: str, approvals: ApprovalsC self.get_command_name(), f'Record {record_uid} is not a typed record and cannot store approvals config', ) - record.custom = merge_approvals_custom_fields(record.custom, approvals) + profile = self.get_approvals_profile() + record.custom = merge_approvals_custom_fields(record.custom, approvals, profile) record_management.update_record(params, record) params.sync_data = True api.sync_down(params) @@ -435,6 +436,7 @@ def _execute_sync_down(self, params, record_uid: str, sync_vault: bool = True) - update_record=lambda uid, approvals: self._patch_record_approvals(params, uid, approvals), command_name=self.get_command_name(), sync_vault=sync_vault, + profile=self.get_approvals_profile(), ) # -- Docker Compose update ----------------------------------------- diff --git a/keepercommander/service/docker/models.py b/keepercommander/service/docker/models.py index c17950fe7..4c263602c 100644 --- a/keepercommander/service/docker/models.py +++ b/keepercommander/service/docker/models.py @@ -177,7 +177,7 @@ class GChatConfig: google_project_id: str google_subscription_id: str google_topic_id: str - chat_approvals_space_id: str + approvals: ApprovalsConfig chat_command_request_record_id: str = GChatConstants.DEFAULT_COMMAND_REQUEST_RECORD_ID chat_command_request_folder_id: str = GChatConstants.DEFAULT_COMMAND_REQUEST_FOLDER_ID chat_command_external_share_id: str = GChatConstants.DEFAULT_COMMAND_EXTERNAL_SHARE_ID @@ -186,3 +186,7 @@ class GChatConfig: pedm_polling_interval: int = 120 device_approval_enabled: bool = False device_approval_polling_interval: int = 120 + + @property + def chat_approvals_space_id(self) -> str: + return self.approvals.single_channel_id diff --git a/unit-tests/service/test_docker_setup_ownership.py b/unit-tests/service/test_docker_setup_ownership.py index 2dbf4cb02..e109d0726 100644 --- a/unit-tests/service/test_docker_setup_ownership.py +++ b/unit-tests/service/test_docker_setup_ownership.py @@ -312,6 +312,63 @@ def test_find_record_in_folder_skips_non_owned(self, mock_get): 'OWNED_REC', ) + def test_gchat_find_folder_uid_by_name_skips_non_owned(self): + from keepercommander.service.commands.integrations.gchat_app_setup import ( + GChatAppSetupCommand, + ) + + params = _params(shared_folder_cache={ + 'ATTACKER_SF': { + 'name': FOLDER_NAME, + 'owner_username': 'mallory@corp.example', + }, + 'OWNED_SF': { + 'name': FOLDER_NAME, + 'owner_username': 'operator@corp.example', + }, + }) + self.assertEqual( + GChatAppSetupCommand()._find_folder_uid_by_name(params, FOLDER_NAME), + 'OWNED_SF', + ) + + def test_gchat_find_folder_uid_by_name_returns_none_for_squat_only(self): + from keepercommander.service.commands.integrations.gchat_app_setup import ( + GChatAppSetupCommand, + ) + + params = _params(shared_folder_cache={ + 'ATTACKER_SF': { + 'name': FOLDER_NAME, + 'owner_username': 'mallory@corp.example', + }, + }) + self.assertIsNone( + GChatAppSetupCommand()._find_folder_uid_by_name(params, FOLDER_NAME) + ) + + @patch('keepercommander.service.commands.integrations.integration_setup_base.api.get_record') + def test_gchat_find_record_in_folder_skips_non_owned(self, mock_get): + from keepercommander.service.commands.integrations.gchat_app_setup import ( + GChatAppSetupCommand, + ) + + owned = MagicMock(title=RECORD_NAME) + shared = MagicMock(title=RECORD_NAME) + mock_get.side_effect = lambda _p, uid: shared if uid == 'SHARED_REC' else owned + + params = _params( + subfolder_record_cache={'FOLDER': ['SHARED_REC', 'OWNED_REC']}, + record_owner_cache={ + 'SHARED_REC': RecordOwner(False, 'attacker'), + 'OWNED_REC': RecordOwner(True, 'operator'), + }, + ) + self.assertEqual( + GChatAppSetupCommand()._find_record_in_folder(params, 'FOLDER', RECORD_NAME), + 'OWNED_REC', + ) + if __name__ == '__main__': unittest.main() diff --git a/unit-tests/service/test_gchat_app_setup.py b/unit-tests/service/test_gchat_app_setup.py index 5f44fbee3..418b59f6d 100644 --- a/unit-tests/service/test_gchat_app_setup.py +++ b/unit-tests/service/test_gchat_app_setup.py @@ -4,7 +4,7 @@ import unittest from keepercommander.service.commands.integrations.gchat_app_setup import GChatAppSetupCommand -from keepercommander.service.docker import GChatConfig, GChatConstants +from keepercommander.service.docker import GChatConfig, GChatConstants, ApprovalsConfig def _valid_service_account(**overrides): @@ -149,19 +149,51 @@ def test_normalize_topic_missing(self): self.assertIsNone(value) self.assertIn('required', error.lower()) - def test_space_id_validation(self): - self.assertTrue(self.cmd._is_valid_space_id('spaces/AAAA')) - self.assertFalse(self.cmd._is_valid_space_id('spaces/')) - self.assertFalse(self.cmd._is_valid_space_id('AAAA')) - self.assertFalse(self.cmd._is_valid_space_id('')) + def test_chat_approvals_space_id_property(self): + from keepercommander.service.docker import ApprovalsConfig + config = GChatConfig( + google_service_account_json='{"type":"service_account"}', + google_project_id='my-gcp-project', + google_subscription_id='keeper-chat-events', + google_topic_id='keeper-chat-topic', + approvals=ApprovalsConfig(multi_channel_enabled=False, single_channel_id='spaces/TEST'), + ) + self.assertEqual(config.chat_approvals_space_id, 'spaces/TEST') + self.assertEqual(config.chat_approvals_space_id, config.approvals.single_channel_id) + + def test_multi_channel_approval_config_serialization(self): + from keepercommander.service.docker import ApprovalsConfig, ApproverTeam + from keepercommander.service.commands.integrations.approvals_setup import approvals_config_to_record_fields + + config = ApprovalsConfig( + multi_channel_enabled=True, + single_channel_id='spaces/DEFAULT', + teams=[ + ApproverTeam(team_uid='team1', name='Team A', channel_id='spaces/A'), + ApproverTeam(team_uid='team2', name='Team B', channel_id='spaces/B', + folder_uids=['folder1']), + ] + ) + fields = { + field.label: field.get_default_value() + for field in approvals_config_to_record_fields(config, self.cmd.get_approvals_profile()) + } + self.assertEqual(fields['multi_channel_approvers_enabled'], 'true') + self.assertEqual(fields['chat_approvals_space_id'], 'spaces/DEFAULT') + teams_json = json.loads(fields['approvals_teams']) + self.assertEqual(len(teams_json), 2) + self.assertEqual(teams_json[0]['team_uid'], 'team1') + self.assertEqual(teams_json[0]['space_id'], 'spaces/A') + self.assertEqual(teams_json[1]['folder_uids'], ['folder1']) def test_build_record_custom_fields(self): + from keepercommander.service.docker import ApprovalsConfig config = GChatConfig( google_service_account_json='{"type":"service_account"}', google_project_id='my-gcp-project', google_subscription_id='keeper-chat-events', google_topic_id='keeper-chat-topic', - chat_approvals_space_id='spaces/AAAA', + approvals=ApprovalsConfig(multi_channel_enabled=False, single_channel_id='spaces/AAAA'), chat_command_request_record_id='1', chat_command_request_folder_id='2', chat_command_external_share_id='3', @@ -182,7 +214,7 @@ def test_build_record_custom_fields(self): self.assertEqual(fields[GChatConstants.FIELD_PROJECT_ID], 'my-gcp-project') self.assertEqual(fields[GChatConstants.FIELD_SUBSCRIPTION_ID], 'keeper-chat-events') self.assertEqual(fields[GChatConstants.FIELD_TOPIC_ID], 'keeper-chat-topic') - self.assertEqual(fields[GChatConstants.FIELD_APPROVALS_SPACE_ID], 'spaces/AAAA') + self.assertEqual(fields['chat_approvals_space_id'], 'spaces/AAAA') self.assertEqual(fields[GChatConstants.FIELD_COMMAND_REQUEST_RECORD_ID], '1') self.assertEqual(fields[GChatConstants.FIELD_COMMAND_REQUEST_FOLDER_ID], '2') self.assertEqual(fields[GChatConstants.FIELD_COMMAND_EXTERNAL_SHARE_ID], '3') @@ -191,6 +223,34 @@ def test_build_record_custom_fields(self): self.assertEqual(fields[GChatConstants.FIELD_PEDM_POLLING_INTERVAL], '60') self.assertEqual(fields[GChatConstants.FIELD_DEVICE_APPROVAL_ENABLED], 'false') + def test_get_approvals_profile(self): + profile = self.cmd.get_approvals_profile() + self.assertIsNotNone(profile) + self.assertEqual(profile.channel_header, 'APPROVALS_SPACE_ID') + self.assertEqual(profile.channel_prompt, 'Space ID (starts with spaces/):') + self.assertTrue(profile.validate_channel('spaces/AAAA')) + self.assertFalse(profile.validate_channel('invalid')) + + def test_build_record_custom_fields_includes_approvals_fields(self): + from keepercommander.service.docker import ApprovalsConfig + config = GChatConfig( + google_service_account_json='{"type":"service_account"}', + google_project_id='my-gcp-project', + google_subscription_id='keeper-chat-events', + google_topic_id='keeper-chat-topic', + approvals=ApprovalsConfig(multi_channel_enabled=False, single_channel_id='spaces/AAAA'), + ) + fields = { + field.label: field.get_default_value() + for field in self.cmd.build_record_custom_fields(config) + } + self.assertIn('multi_channel_approvers_enabled', fields) + self.assertIn('chat_approvals_space_id', fields) + self.assertIn('approvals_teams', fields) + self.assertEqual(fields['multi_channel_approvers_enabled'], 'false') + self.assertEqual(fields['chat_approvals_space_id'], 'spaces/AAAA') + self.assertEqual(fields['approvals_teams'], '') + if __name__ == '__main__': unittest.main()