diff --git a/awx/main/management/commands/import_auth_config_to_gateway.py b/awx/main/management/commands/import_auth_config_to_gateway.py index 89588f08b2..babe849092 100644 --- a/awx/main/management/commands/import_auth_config_to_gateway.py +++ b/awx/main/management/commands/import_auth_config_to_gateway.py @@ -60,6 +60,7 @@ class Command(BaseCommand): 'created': 0, 'failed': 0, 'mappers_created': 0, + 'mappers_updated': 0, 'mappers_failed': 0, } @@ -71,7 +72,7 @@ class Command(BaseCommand): result = migrator.migrate() self._print_export_summary(migrator.get_authenticator_type(), result) - # Accumulate results + # Accumulate results - handle missing keys gracefully for key in total_results: total_results[key] += result.get(key, 0) @@ -80,6 +81,7 @@ class Command(BaseCommand): self.stdout.write(f'Total authenticators created: {total_results["created"]}') self.stdout.write(f'Total authenticators failed: {total_results["failed"]}') self.stdout.write(f'Total mappers created: {total_results["mappers_created"]}') + self.stdout.write(f'Total mappers updated: {total_results["mappers_updated"]}') self.stdout.write(f'Total mappers failed: {total_results["mappers_failed"]}') except GatewayAPIError as e: @@ -96,7 +98,8 @@ class Command(BaseCommand): def _print_export_summary(self, config_type, result): """Print a summary of the export results.""" self.stdout.write(f'\n--- {config_type} Export Summary ---') - self.stdout.write(f'Authenticators created: {result["created"]}') - self.stdout.write(f'Authenticators failed: {result["failed"]}') - self.stdout.write(f'Mappers created: {result["mappers_created"]}') - self.stdout.write(f'Mappers failed: {result["mappers_failed"]}') + self.stdout.write(f'Authenticators created: {result.get("created", 0)}') + self.stdout.write(f'Authenticators failed: {result.get("failed", 0)}') + self.stdout.write(f'Mappers created: {result.get("mappers_created", 0)}') + self.stdout.write(f'Mappers updated: {result.get("mappers_updated", 0)}') + self.stdout.write(f'Mappers failed: {result.get("mappers_failed", 0)}') diff --git a/awx/main/tests/unit/utils/test_base_migrator.py b/awx/main/tests/unit/utils/test_base_migrator.py new file mode 100644 index 0000000000..f12cd5ccda --- /dev/null +++ b/awx/main/tests/unit/utils/test_base_migrator.py @@ -0,0 +1,628 @@ +""" +Unit tests for base authenticator migrator functionality. +""" + +import pytest +from unittest.mock import Mock +from awx.sso.utils.base_migrator import BaseAuthenticatorMigrator + + +class TestBaseAuthenticatorMigrator: + """Tests for BaseAuthenticatorMigrator class.""" + + def setup_method(self): + """Set up test fixtures.""" + self.gateway_client = Mock() + self.command = Mock() + self.migrator = BaseAuthenticatorMigrator(self.gateway_client, self.command) + + def test_generate_authenticator_slug(self): + """Test slug generation is deterministic.""" + slug1 = self.migrator._generate_authenticator_slug('github', 'github-org', 'client123') + slug2 = self.migrator._generate_authenticator_slug('github', 'github-org', 'client123') + + assert slug1 == slug2 + assert slug1.startswith('awx-github-') + assert len(slug1.split('-')[-1]) == 8 # Hash should be 8 characters + + def test_generate_authenticator_slug_different_inputs(self): + """Test that different inputs generate different slugs.""" + slug1 = self.migrator._generate_authenticator_slug('github', 'github-org', 'client123') + slug2 = self.migrator._generate_authenticator_slug('github', 'github-org', 'client456') + slug3 = self.migrator._generate_authenticator_slug('ldap', 'ldap', 'ldap://server') + + assert slug1 != slug2 + assert slug1 != slug3 + assert slug2 != slug3 + + def test_get_mapper_ignore_keys_default(self): + """Test default mapper ignore keys.""" + ignore_keys = self.migrator._get_mapper_ignore_keys() + + expected_keys = ['id', 'authenticator', 'created', 'modified', 'summary_fields', 'modified_by', 'created_by', 'related', 'url'] + assert ignore_keys == expected_keys + + +class TestAuthenticatorConfigComparison: + """Tests for authenticator configuration comparison methods.""" + + def setup_method(self): + """Set up test fixtures.""" + self.gateway_client = Mock() + self.command = Mock() + self.migrator = BaseAuthenticatorMigrator(self.gateway_client, self.command) + + def test_authenticator_configs_match_identical(self): + """Test that identical configurations match.""" + existing_auth = { + 'name': 'GitHub Auth', + 'type': 'ansible_base.authentication.authenticator_plugins.github', + 'enabled': True, + 'create_objects': True, + 'remove_users': False, + 'configuration': {'KEY': 'client123', 'SECRET': 'secret456'}, + } + + new_config = existing_auth.copy() + new_config['configuration'] = existing_auth['configuration'].copy() + + assert self.migrator._authenticator_configs_match(existing_auth, new_config) == (True, []) + + def test_authenticator_configs_match_with_ignore_keys(self): + """Test that configurations match when ignoring specified keys.""" + existing_auth = { + 'name': 'GitHub Auth', + 'type': 'ansible_base.authentication.authenticator_plugins.github', + 'enabled': True, + 'create_objects': True, + 'remove_users': False, + 'configuration': {'KEY': 'client123', 'SECRET': 'secret456', 'CALLBACK_URL': 'https://gateway.example.com/callback'}, + } + + new_config = { + 'name': 'GitHub Auth', + 'type': 'ansible_base.authentication.authenticator_plugins.github', + 'enabled': True, + 'create_objects': True, + 'remove_users': False, + 'configuration': {'KEY': 'client123', 'SECRET': 'secret456'}, + } + + # Should not match without ignore keys + assert self.migrator._authenticator_configs_match(existing_auth, new_config) == ( + False, + [' CALLBACK_URL: existing="https://gateway.example.com/callback" vs new='], + ) + + # Should match when ignoring CALLBACK_URL + ignore_keys = ['CALLBACK_URL'] + assert self.migrator._authenticator_configs_match(existing_auth, new_config, ignore_keys) == (True, []) + + def test_authenticator_configs_different_basic_fields(self): + """Test that configurations don't match when basic fields differ.""" + existing_auth = { + 'name': 'GitHub Auth', + 'type': 'ansible_base.authentication.authenticator_plugins.github', + 'enabled': True, + 'create_objects': True, + 'remove_users': False, + 'configuration': {'KEY': 'client123', 'SECRET': 'secret456'}, + } + + # Test different name + new_config = existing_auth.copy() + new_config['name'] = 'Different GitHub Auth' + match, differences = self.migrator._authenticator_configs_match(existing_auth, new_config) + assert match is False + assert len(differences) == 1 + assert 'name:' in differences[0] + + # Test different type + new_config = existing_auth.copy() + new_config['type'] = 'ansible_base.authentication.authenticator_plugins.ldap' + match, differences = self.migrator._authenticator_configs_match(existing_auth, new_config) + assert match is False + assert len(differences) == 1 + assert 'type:' in differences[0] + + # Test different enabled + new_config = existing_auth.copy() + new_config['enabled'] = False + match, differences = self.migrator._authenticator_configs_match(existing_auth, new_config) + assert match is False + assert len(differences) == 1 + assert 'enabled:' in differences[0] + + def test_authenticator_configs_different_configuration(self): + """Test that configurations don't match when configuration section differs.""" + existing_auth = { + 'name': 'GitHub Auth', + 'type': 'ansible_base.authentication.authenticator_plugins.github', + 'enabled': True, + 'create_objects': True, + 'remove_users': False, + 'configuration': {'KEY': 'client123', 'SECRET': 'secret456', 'SCOPE': 'read:org'}, + } + + # Test different KEY + new_config = existing_auth.copy() + new_config['configuration'] = {'KEY': 'client789', 'SECRET': 'secret456', 'SCOPE': 'read:org'} + match, differences = self.migrator._authenticator_configs_match(existing_auth, new_config) + assert match is False + assert len(differences) == 1 + assert 'KEY:' in differences[0] + assert 'existing="client123"' in differences[0] + assert 'new="client789"' in differences[0] + + # Test missing key in new config + new_config = existing_auth.copy() + new_config['configuration'] = {'KEY': 'client123', 'SECRET': 'secret456'} + match, differences = self.migrator._authenticator_configs_match(existing_auth, new_config) + assert match is False + assert len(differences) == 1 + assert 'SCOPE:' in differences[0] + assert 'vs new=' in differences[0] + + # Test extra key in new config + new_config = existing_auth.copy() + new_config['configuration'] = {'KEY': 'client123', 'SECRET': 'secret456', 'SCOPE': 'read:org', 'EXTRA_KEY': 'extra_value'} + match, differences = self.migrator._authenticator_configs_match(existing_auth, new_config) + assert match is False + assert len(differences) == 1 + assert 'EXTRA_KEY:' in differences[0] + assert 'existing=' in differences[0] + + def test_authenticator_configs_differences_details(self): + """Test that difference tracking provides detailed information.""" + existing_auth = { + 'name': 'GitHub Auth', + 'type': 'ansible_base.authentication.authenticator_plugins.github', + 'enabled': True, + 'create_objects': True, + 'remove_users': False, + 'configuration': {'KEY': 'client123', 'SECRET': 'secret456', 'SCOPE': 'read:org', 'CALLBACK_URL': 'https://gateway.example.com/callback'}, + } + + # Test multiple differences with ignore keys + new_config = { + 'name': 'GitHub Auth', + 'type': 'ansible_base.authentication.authenticator_plugins.github', + 'enabled': True, + 'create_objects': True, + 'remove_users': False, + 'configuration': { + 'KEY': 'client456', # Different value + 'SECRET': 'newsecret', # Different value + 'SCOPE': 'read:org', # Same value + # CALLBACK_URL missing (but ignored) + 'NEW_FIELD': 'new_value', # Extra field + }, + } + + ignore_keys = ['CALLBACK_URL'] + match, differences = self.migrator._authenticator_configs_match(existing_auth, new_config, ignore_keys) + + assert match is False + assert len(differences) == 3 # KEY, SECRET, NEW_FIELD + + # Check that all expected differences are captured + difference_text = ' '.join(differences) + assert 'KEY:' in difference_text + assert 'SECRET:' in difference_text + assert 'NEW_FIELD:' in difference_text + assert 'CALLBACK_URL' not in difference_text # Should be ignored + + +class TestMapperComparison: + """Tests for mapper comparison methods.""" + + def setup_method(self): + """Set up test fixtures.""" + self.gateway_client = Mock() + self.command = Mock() + self.migrator = BaseAuthenticatorMigrator(self.gateway_client, self.command) + + def test_mappers_match_structurally_identical(self): + """Test that identical mappers match structurally.""" + mapper1 = {'organization': 'myorg', 'team': 'engineering', 'map_type': 'team', 'role': 'Team Member'} + + mapper2 = mapper1.copy() + + assert self.migrator._mappers_match_structurally(mapper1, mapper2) is True + + def test_mappers_match_structurally_different_fields(self): + """Test that mappers don't match structurally when key fields differ.""" + base_mapper = {'organization': 'myorg', 'team': 'engineering', 'map_type': 'team', 'role': 'Team Member'} + + # Test different organization + mapper2 = base_mapper.copy() + mapper2['organization'] = 'otherorg' + assert self.migrator._mappers_match_structurally(base_mapper, mapper2) is False + + # Test different team + mapper2 = base_mapper.copy() + mapper2['team'] = 'qa' + assert self.migrator._mappers_match_structurally(base_mapper, mapper2) is False + + # Test different map_type + mapper2 = base_mapper.copy() + mapper2['map_type'] = 'organization' + assert self.migrator._mappers_match_structurally(base_mapper, mapper2) is False + + # Test different role + mapper2 = base_mapper.copy() + mapper2['role'] = 'Organization Admin' + assert self.migrator._mappers_match_structurally(base_mapper, mapper2) is False + + def test_mapper_configs_match_identical(self): + """Test that identical mapper configurations match.""" + mapper1 = { + 'name': 'myorg - engineering', + 'organization': 'myorg', + 'team': 'engineering', + 'map_type': 'team', + 'role': 'Team Member', + 'order': 1, + 'triggers': {'groups': {'has_or': ['engineers']}}, + 'revoke': False, + } + + mapper2 = mapper1.copy() + + assert self.migrator._mapper_configs_match(mapper1, mapper2) is True + + def test_mapper_configs_match_with_ignore_keys(self): + """Test that mapper configurations match when ignoring specified keys.""" + existing_mapper = { + 'id': 123, + 'authenticator': 456, + 'name': 'myorg - engineering', + 'organization': 'myorg', + 'team': 'engineering', + 'map_type': 'team', + 'role': 'Team Member', + 'order': 1, + 'triggers': {'groups': {'has_or': ['engineers']}}, + 'revoke': False, + 'created': '2023-01-01T00:00:00Z', + 'modified': '2023-01-01T00:00:00Z', + } + + new_mapper = { + 'name': 'myorg - engineering', + 'organization': 'myorg', + 'team': 'engineering', + 'map_type': 'team', + 'role': 'Team Member', + 'order': 1, + 'triggers': {'groups': {'has_or': ['engineers']}}, + 'revoke': False, + } + + # Should not match without ignore keys + assert self.migrator._mapper_configs_match(existing_mapper, new_mapper) is False + + # Should match when ignoring auto-generated fields + ignore_keys = ['id', 'authenticator', 'created', 'modified'] + assert self.migrator._mapper_configs_match(existing_mapper, new_mapper, ignore_keys) is True + + def test_mapper_configs_different_values(self): + """Test that mapper configurations don't match when values differ.""" + mapper1 = { + 'name': 'myorg - engineering', + 'organization': 'myorg', + 'team': 'engineering', + 'map_type': 'team', + 'role': 'Team Member', + 'order': 1, + 'triggers': {'groups': {'has_or': ['engineers']}}, + 'revoke': False, + } + + # Test different name + mapper2 = mapper1.copy() + mapper2['name'] = 'myorg - qa' + assert self.migrator._mapper_configs_match(mapper1, mapper2) is False + + # Test different order + mapper2 = mapper1.copy() + mapper2['order'] = 2 + assert self.migrator._mapper_configs_match(mapper1, mapper2) is False + + # Test different triggers + mapper2 = mapper1.copy() + mapper2['triggers'] = {'groups': {'has_or': ['qa-team']}} + assert self.migrator._mapper_configs_match(mapper1, mapper2) is False + + # Test different revoke + mapper2 = mapper1.copy() + mapper2['revoke'] = True + assert self.migrator._mapper_configs_match(mapper1, mapper2) is False + + +class TestCompareMapperLists: + """Tests for _compare_mapper_lists method.""" + + def setup_method(self): + """Set up test fixtures.""" + self.gateway_client = Mock() + self.command = Mock() + self.migrator = BaseAuthenticatorMigrator(self.gateway_client, self.command) + + def test_compare_mapper_lists_empty(self): + """Test comparing empty mapper lists.""" + existing_mappers = [] + new_mappers = [] + + mappers_to_update, mappers_to_create = self.migrator._compare_mapper_lists(existing_mappers, new_mappers) + + assert mappers_to_update == [] + assert mappers_to_create == [] + + def test_compare_mapper_lists_all_new(self): + """Test when all new mappers need to be created.""" + existing_mappers = [] + new_mappers = [ + { + 'name': 'myorg - engineering', + 'organization': 'myorg', + 'team': 'engineering', + 'map_type': 'team', + 'role': 'Team Member', + 'order': 1, + 'triggers': {'groups': {'has_or': ['engineers']}}, + 'revoke': False, + }, + { + 'name': 'myorg - qa', + 'organization': 'myorg', + 'team': 'qa', + 'map_type': 'team', + 'role': 'Team Member', + 'order': 2, + 'triggers': {'groups': {'has_or': ['qa-team']}}, + 'revoke': False, + }, + ] + + mappers_to_update, mappers_to_create = self.migrator._compare_mapper_lists(existing_mappers, new_mappers) + + assert mappers_to_update == [] + assert mappers_to_create == new_mappers + + def test_compare_mapper_lists_all_existing_match(self): + """Test when all existing mappers match exactly.""" + existing_mappers = [ + { + 'id': 123, + 'authenticator': 456, + 'name': 'myorg - engineering', + 'organization': 'myorg', + 'team': 'engineering', + 'map_type': 'team', + 'role': 'Team Member', + 'order': 1, + 'triggers': {'groups': {'has_or': ['engineers']}}, + 'revoke': False, + 'created': '2023-01-01T00:00:00Z', + 'modified': '2023-01-01T00:00:00Z', + } + ] + + new_mappers = [ + { + 'name': 'myorg - engineering', + 'organization': 'myorg', + 'team': 'engineering', + 'map_type': 'team', + 'role': 'Team Member', + 'order': 1, + 'triggers': {'groups': {'has_or': ['engineers']}}, + 'revoke': False, + } + ] + + ignore_keys = ['id', 'authenticator', 'created', 'modified'] + mappers_to_update, mappers_to_create = self.migrator._compare_mapper_lists(existing_mappers, new_mappers, ignore_keys) + + assert mappers_to_update == [] + assert mappers_to_create == [] + + def test_compare_mapper_lists_needs_update(self): + """Test when existing mappers need updates.""" + existing_mappers = [ + { + 'id': 123, + 'authenticator': 456, + 'name': 'myorg - engineering', + 'organization': 'myorg', + 'team': 'engineering', + 'map_type': 'team', + 'role': 'Team Member', + 'order': 1, + 'triggers': {'groups': {'has_or': ['old-engineers']}}, + 'revoke': False, + 'created': '2023-01-01T00:00:00Z', + 'modified': '2023-01-01T00:00:00Z', + } + ] + + new_mappers = [ + { + 'name': 'myorg - engineering', + 'organization': 'myorg', + 'team': 'engineering', + 'map_type': 'team', + 'role': 'Team Member', + 'order': 1, + 'triggers': {'groups': {'has_or': ['new-engineers']}}, + 'revoke': False, + } + ] + + ignore_keys = ['id', 'authenticator', 'created', 'modified'] + mappers_to_update, mappers_to_create = self.migrator._compare_mapper_lists(existing_mappers, new_mappers, ignore_keys) + + assert len(mappers_to_update) == 1 + assert mappers_to_update[0] == (existing_mappers[0], new_mappers[0]) + assert mappers_to_create == [] + + def test_compare_mapper_lists_mixed_operations(self): + """Test mix of updates and creates.""" + existing_mappers = [ + { + 'id': 123, + 'authenticator': 456, + 'name': 'myorg - engineering', + 'organization': 'myorg', + 'team': 'engineering', + 'map_type': 'team', + 'role': 'Team Member', + 'order': 1, + 'triggers': {'groups': {'has_or': ['old-engineers']}}, + 'revoke': False, + 'created': '2023-01-01T00:00:00Z', + 'modified': '2023-01-01T00:00:00Z', + } + ] + + new_mappers = [ + { + 'name': 'myorg - engineering', + 'organization': 'myorg', + 'team': 'engineering', + 'map_type': 'team', + 'role': 'Team Member', + 'order': 1, + 'triggers': {'groups': {'has_or': ['new-engineers']}}, + 'revoke': False, + }, + { + 'name': 'myorg - qa', + 'organization': 'myorg', + 'team': 'qa', + 'map_type': 'team', + 'role': 'Team Member', + 'order': 2, + 'triggers': {'groups': {'has_or': ['qa-team']}}, + 'revoke': False, + }, + ] + + ignore_keys = ['id', 'authenticator', 'created', 'modified'] + mappers_to_update, mappers_to_create = self.migrator._compare_mapper_lists(existing_mappers, new_mappers, ignore_keys) + + assert len(mappers_to_update) == 1 + assert mappers_to_update[0] == (existing_mappers[0], new_mappers[0]) + assert len(mappers_to_create) == 1 + assert mappers_to_create[0] == new_mappers[1] + + def test_compare_mapper_lists_no_structural_match(self): + """Test when existing and new mappers don't match structurally.""" + existing_mappers = [ + { + 'id': 123, + 'authenticator': 456, + 'name': 'myorg - engineering', + 'organization': 'myorg', + 'team': 'engineering', + 'map_type': 'team', + 'role': 'Team Member', + 'order': 1, + 'triggers': {'groups': {'has_or': ['engineers']}}, + 'revoke': False, + } + ] + + new_mappers = [ + { + 'name': 'otherorg - qa', + 'organization': 'otherorg', # Different organization + 'team': 'qa', # Different team + 'map_type': 'team', + 'role': 'Team Member', + 'order': 1, + 'triggers': {'groups': {'has_or': ['qa-team']}}, + 'revoke': False, + } + ] + + mappers_to_update, mappers_to_create = self.migrator._compare_mapper_lists(existing_mappers, new_mappers) + + assert mappers_to_update == [] + assert mappers_to_create == new_mappers + + +# Parametrized tests for edge cases +@pytest.mark.parametrize( + "existing_auth,new_config,ignore_keys,expected_match,expected_differences_count", + [ + # Test with None values + ({'name': 'Test', 'configuration': {'KEY': None}}, {'name': 'Test', 'configuration': {'KEY': None}}, [], True, 0), + # Test with empty configuration + ({'name': 'Test', 'configuration': {}}, {'name': 'Test', 'configuration': {}}, [], True, 0), + # Test missing configuration section + ({'name': 'Test'}, {'name': 'Test'}, [], True, 0), + # Test with ignore keys matching + ( + {'name': 'Test', 'configuration': {'KEY': 'value', 'IGNORE_ME': 'old'}}, + {'name': 'Test', 'configuration': {'KEY': 'value', 'IGNORE_ME': 'new'}}, + ['IGNORE_ME'], + True, + 0, + ), + # Test with differences that are not ignored + ( + {'name': 'Test', 'configuration': {'KEY': 'value1'}}, + {'name': 'Test', 'configuration': {'KEY': 'value2'}}, + [], + False, + 1, + ), + ], +) +def test_authenticator_configs_match_edge_cases(existing_auth, new_config, ignore_keys, expected_match, expected_differences_count): + """Test edge cases for authenticator configuration matching.""" + gateway_client = Mock() + command = Mock() + migrator = BaseAuthenticatorMigrator(gateway_client, command) + + match, differences = migrator._authenticator_configs_match(existing_auth, new_config, ignore_keys) + assert match == expected_match + assert len(differences) == expected_differences_count + + +@pytest.mark.parametrize( + "mapper1,mapper2,ignore_keys,expected", + [ + # Test with None team values (org mappers) + ( + {'organization': 'myorg', 'team': None, 'map_type': 'organization', 'role': 'Organization Admin'}, + {'organization': 'myorg', 'team': None, 'map_type': 'organization', 'role': 'Organization Admin'}, + [], + True, + ), + # Test with ignore keys (for structural matching, ignore_keys shouldn't matter) + ( + {'organization': 'myorg', 'team': 'eng', 'map_type': 'team', 'role': 'Team Member', 'id': 123}, + {'organization': 'myorg', 'team': 'eng', 'map_type': 'team', 'role': 'Team Member', 'id': 456}, + ['id'], + True, + ), + # Test structural mismatch + ( + {'organization': 'myorg', 'team': 'eng', 'map_type': 'team', 'role': 'Team Member'}, + {'organization': 'myorg', 'team': 'qa', 'map_type': 'team', 'role': 'Team Member'}, + [], + False, + ), + ], +) +def test_mappers_match_structurally_edge_cases(mapper1, mapper2, ignore_keys, expected): + """Test edge cases for mapper structural matching.""" + gateway_client = Mock() + command = Mock() + migrator = BaseAuthenticatorMigrator(gateway_client, command) + + result = migrator._mappers_match_structurally(mapper1, mapper2, ignore_keys) + assert result == expected diff --git a/awx/main/utils/gateway_client.py b/awx/main/utils/gateway_client.py index 5bc7add294..b7ceaca97f 100644 --- a/awx/main/utils/gateway_client.py +++ b/awx/main/utils/gateway_client.py @@ -170,6 +170,41 @@ class GatewayClient: except requests.RequestException as e: raise GatewayAPIError(f"Failed to create authenticator: {str(e)}") + def update_authenticator(self, authenticator_id: int, authenticator_config: Dict[str, Any]) -> Dict[str, Any]: + """Update an existing authenticator in Gateway. + + Args: + authenticator_id: ID of the authenticator to update + authenticator_config: Authenticator configuration dictionary + + Returns: + dict: Updated authenticator data + + Raises: + GatewayAPIError: If update fails + """ + endpoint = f'/api/gateway/v1/authenticators/{authenticator_id}/' + + try: + response = self._make_request('PATCH', endpoint, data=authenticator_config) + + if response.status_code == 200: + result = response.json() + logger.info(f"Successfully updated authenticator: {result.get('name', 'Unknown')}") + return result + else: + error_msg = f"Failed to update authenticator. Status: {response.status_code}" + try: + error_data = response.json() + error_msg += f", Error: {error_data}" + except requests.exceptions.JSONDecodeError: + error_msg += f", Response: {response.text}" + + raise GatewayAPIError(error_msg, response.status_code, response.json() if response.content else None) + + except requests.RequestException as e: + raise GatewayAPIError(f"Failed to update authenticator: {str(e)}") + def create_authenticator_map(self, authenticator_id: int, mapper_config: Dict[str, Any]) -> Dict[str, Any]: """Create a new authenticator map in Gateway. @@ -206,6 +241,41 @@ class GatewayClient: except requests.RequestException as e: raise GatewayAPIError(f"Failed to create authenticator map: {str(e)}") + def update_authenticator_map(self, mapper_id: int, mapper_config: Dict[str, Any]) -> Dict[str, Any]: + """Update an existing authenticator map in Gateway. + + Args: + mapper_id: ID of the authenticator map to update + mapper_config: Mapper configuration dictionary + + Returns: + dict: Updated mapper data + + Raises: + GatewayAPIError: If update fails + """ + endpoint = f'/api/gateway/v1/authenticator_maps/{mapper_id}/' + + try: + response = self._make_request('PATCH', endpoint, data=mapper_config) + + if response.status_code == 200: + result = response.json() + logger.info(f"Successfully updated authenticator map: {result.get('name', 'Unknown')}") + return result + else: + error_msg = f"Failed to update authenticator map. Status: {response.status_code}" + try: + error_data = response.json() + error_msg += f", Error: {error_data}" + except requests.exceptions.JSONDecodeError: + error_msg += f", Response: {response.text}" + + raise GatewayAPIError(error_msg, response.status_code, response.json() if response.content else None) + + except requests.RequestException as e: + raise GatewayAPIError(f"Failed to update authenticator map: {str(e)}") + def get_authenticators(self, params: Optional[Dict] = None) -> List[Dict[str, Any]]: """Get list of authenticators from Gateway. @@ -236,6 +306,33 @@ class GatewayClient: except requests.RequestException as e: raise GatewayAPIError(f"Failed to get authenticators: {str(e)}") + def get_authenticator_by_slug(self, slug: str) -> Optional[Dict[str, Any]]: + """Get a specific authenticator by slug. + + Args: + slug: The authenticator slug to search for + + Returns: + dict: The authenticator data if found, None otherwise + + Raises: + GatewayAPIError: If request fails + """ + try: + # Use query parameter to filter by slug - more efficient than getting all + authenticators = self.get_authenticators(params={'slug': slug}) + + # Return the first match (slugs should be unique) + if authenticators: + return authenticators[0] + return None + + except GatewayAPIError as e: + # Re-raise Gateway API errors + raise e + except Exception as e: + raise GatewayAPIError(f"Failed to get authenticator by slug: {str(e)}") + def get_authenticator_maps(self, authenticator_id: int) -> List[Dict[str, Any]]: """Get list of maps for a specific authenticator. @@ -248,7 +345,7 @@ class GatewayClient: Raises: GatewayAPIError: If request fails """ - endpoint = f'/api/gateway/v1/authenticators/{authenticator_id}/maps/' + endpoint = f'/api/gateway/v1/authenticators/{authenticator_id}/authenticator_maps/' try: response = self._make_request('GET', endpoint) diff --git a/awx/sso/utils/base_migrator.py b/awx/sso/utils/base_migrator.py index 38eafc0b7c..49cbd98598 100644 --- a/awx/sso/utils/base_migrator.py +++ b/awx/sso/utils/base_migrator.py @@ -48,20 +48,23 @@ class BaseAuthenticatorMigrator: if self.create_gateway_authenticator(config): created_authenticators.append(config) - # Create mappers for successfully created authenticators + # Process mappers for successfully created/updated authenticators mappers_created = 0 + mappers_updated = 0 mappers_failed = 0 if created_authenticators: - self._write_output('\n=== Creating Authenticator Mappers ===', 'success') + self._write_output('\n=== Processing Authenticator Mappers ===', 'success') for config in created_authenticators: - mapper_result = self._create_gateway_mappers(config) + mapper_result = self._process_gateway_mappers(config) mappers_created += mapper_result['created'] + mappers_updated += mapper_result['updated'] mappers_failed += mapper_result['failed'] return { 'created': len(created_authenticators), 'failed': len(configs) - len(created_authenticators), 'mappers_created': mappers_created, + 'mappers_updated': mappers_updated, 'mappers_failed': mappers_failed, } @@ -105,46 +108,352 @@ class BaseAuthenticatorMigrator: final_slug = f"awx-{auth_type}-{slug_hash}" return final_slug - def _create_gateway_mappers(self, config): - """Create authenticator mappers in Gateway from AWX config.""" + def submit_authenticator(self, gateway_config, ignore_keys=[], config={}): + """ + Submit an authenticator to Gateway - either create new or update existing. + + Args: + gateway_config: Complete Gateway authenticator configuration + ignore_keys: List of configuration keys to ignore during comparison + config: Optional AWX config dict to store result data + + Returns: + bool: True if authenticator was submitted successfully, False otherwise + """ + authenticator_slug = gateway_config.get('slug') + if not authenticator_slug: + self._write_output('Gateway config missing slug, cannot submit authenticator', 'error') + return False + + try: + # Check if authenticator already exists by slug + existing_authenticator = self.gateway_client.get_authenticator_by_slug(authenticator_slug) + + if existing_authenticator: + # Authenticator exists, check if configuration matches + authenticator_id = existing_authenticator.get('id') + + configs_match, differences = self._authenticator_configs_match(existing_authenticator, gateway_config, ignore_keys) + + if configs_match: + self._write_output(f'⚠ Authenticator already exists with matching configuration (ID: {authenticator_id})', 'warning') + # Store the existing result for mapper creation + config['gateway_authenticator_id'] = authenticator_id + config['gateway_authenticator'] = existing_authenticator + return True + else: + self._write_output(f'⚠ Authenticator exists but configuration differs (ID: {authenticator_id})', 'warning') + self._write_output(' Configuration comparison:') + + # Log differences between the existing and the new configuration in case of an update + for difference in differences: + self._write_output(f' {difference}') + + # Update the existing authenticator + self._write_output(' Updating authenticator with new configuration...') + try: + # Don't include the slug in the update since it shouldn't change + update_config = gateway_config.copy() + if 'slug' in update_config: + del update_config['slug'] + + result = self.gateway_client.update_authenticator(authenticator_id, update_config) + self._write_output(f'✓ Successfully updated authenticator with ID: {authenticator_id}', 'success') + + # Store the updated result for mapper creation + config['gateway_authenticator_id'] = authenticator_id + config['gateway_authenticator'] = result + return True + except GatewayAPIError as e: + self._write_output(f'✗ Failed to update authenticator: {e.message}', 'error') + if e.response_data: + self._write_output(f' Details: {e.response_data}', 'error') + return False + else: + # Authenticator doesn't exist, create it + self._write_output('Creating new authenticator...') + + # Create the authenticator + result = self.gateway_client.create_authenticator(gateway_config) + + self._write_output(f'✓ Successfully created authenticator with ID: {result.get("id")}', 'success') + + # Store the result for potential mapper creation later + config['gateway_authenticator_id'] = result.get('id') + config['gateway_authenticator'] = result + return True + + except GatewayAPIError as e: + self._write_output(f'✗ Failed to submit authenticator: {e.message}', 'error') + if e.response_data: + self._write_output(f' Details: {e.response_data}', 'error') + return False + except Exception as e: + self._write_output(f'✗ Unexpected error submitting authenticator: {str(e)}', 'error') + return False + + def _authenticator_configs_match(self, existing_auth, new_config, ignore_keys=[]): + """ + Compare existing authenticator configuration with new configuration. + + Args: + existing_auth: Existing authenticator data from Gateway + new_config: New authenticator configuration to be created + ignore_keys: List of configuration keys to ignore during comparison + (e.g., ['CALLBACK_URL'] for auto-generated fields) + + Returns: + bool: True if configurations match, False otherwise + """ + + # Keep track of the differences between the existing and the new configuration + # Logging them makes debugging much easier + differences = [] + + if existing_auth.get('name') != new_config.get('name'): + differences.append(f' name: existing="{existing_auth.get("name")}" vs new="{new_config.get("name")}"') + elif existing_auth.get('type') != new_config.get('type'): + differences.append(f' type: existing="{existing_auth.get("type")}" vs new="{new_config.get("type")}"') + elif existing_auth.get('enabled') != new_config.get('enabled'): + differences.append(f' enabled: existing="{existing_auth.get("enabled")}" vs new="{new_config.get("enabled")}"') + elif existing_auth.get('create_objects') != new_config.get('create_objects'): + differences.append(f' create_objects: existing="{existing_auth.get("create_objects")}" vs new="{new_config.get("create_objects")}"') + elif existing_auth.get('remove_users') != new_config.get('remove_users'): + differences.append(f' create_objects: existing="{existing_auth.get("remove_users")}" vs new="{new_config.get("remove_users")}"') + + # Compare configuration section + existing_config = existing_auth.get('configuration', {}) + new_config_section = new_config.get('configuration', {}) + + # Helper function to check if a key should be ignored + def should_ignore_key(config_key): + return config_key in ignore_keys + + # Check if all keys in new config exist in existing config with same values + for key, value in new_config_section.items(): + if should_ignore_key(key): + continue + if key not in existing_config: + differences.append(f' {key}: existing= vs new="{value}"') + elif existing_config[key] != value: + differences.append(f' {key}: existing="{existing_config.get(key)}" vs new="{value}"') + + # Check if existing config has extra keys that new config doesn't have + # (this might indicate configuration drift), but ignore keys in ignore_keys + for key in existing_config: + if should_ignore_key(key): + continue + if key not in new_config_section: + differences.append(f' {key}: existing="{existing_config.get(key)}" vs new=') + + return len(differences) == 0, differences + + def _compare_mapper_lists(self, existing_mappers, new_mappers, ignore_keys=None): + """ + Compare existing and new mapper lists to determine which need updates vs creation. + + Args: + existing_mappers: List of existing mapper configurations from Gateway + new_mappers: List of new mapper configurations to be created/updated + ignore_keys: List of keys to ignore during comparison (e.g., auto-generated fields) + + Returns: + tuple: (mappers_to_update, mappers_to_create) + mappers_to_update: List of tuples (existing_mapper, new_mapper) for updates + mappers_to_create: List of new_mapper configs that don't match any existing + """ + if ignore_keys is None: + ignore_keys = [] + + mappers_to_update = [] + mappers_to_create = [] + + for new_mapper in new_mappers: + matched_existing = None + + # Try to find a matching existing mapper + for existing_mapper in existing_mappers: + if self._mappers_match_structurally(existing_mapper, new_mapper, ignore_keys): + matched_existing = existing_mapper + break + + if matched_existing: + # Check if the configuration actually differs (ignoring auto-generated fields) + if not self._mapper_configs_match(matched_existing, new_mapper, ignore_keys): + mappers_to_update.append((matched_existing, new_mapper)) + # If configs match exactly, no action needed (mapper is up to date) + else: + # No matching existing mapper found, needs to be created + mappers_to_create.append(new_mapper) + + return mappers_to_update, mappers_to_create + + def _mappers_match_structurally(self, existing_mapper, new_mapper, ignore_keys=None): + """ + Check if two mappers match structurally (same organization, team, map_type, role). + This identifies if they represent the same logical mapping. + + Args: + existing_mapper: Existing mapper configuration from Gateway + new_mapper: New mapper configuration + ignore_keys: List of keys to ignore during comparison + + Returns: + bool: True if mappers represent the same logical mapping + """ + if ignore_keys is None: + ignore_keys = [] + + # Compare key structural fields that identify the same logical mapper + structural_fields = ['organization', 'team', 'map_type', 'role'] + + for field in structural_fields: + if existing_mapper.get(field) != new_mapper.get(field): + return False + + return True + + def _mapper_configs_match(self, existing_mapper, new_mapper, ignore_keys=None): + """ + Compare mapper configurations to check if they are identical. + + Args: + existing_mapper: Existing mapper configuration from Gateway + new_mapper: New mapper configuration + ignore_keys: List of keys to ignore during comparison + + Returns: + bool: True if configurations match, False otherwise + """ + if ignore_keys is None: + ignore_keys = [] + + # Helper function to check if a key should be ignored + def should_ignore_key(config_key): + return config_key in ignore_keys + + # Compare all mapper fields except ignored ones + all_keys = set(existing_mapper.keys()) | set(new_mapper.keys()) + + for key in all_keys: + if should_ignore_key(key): + continue + + existing_value = existing_mapper.get(key) + new_value = new_mapper.get(key) + + if existing_value != new_value: + return False + + return True + + def _process_gateway_mappers(self, config): + """Process authenticator mappers in Gateway from AWX config - create or update as needed.""" authenticator_id = config.get('gateway_authenticator_id') if not authenticator_id: self._write_output(f'No authenticator ID found for {config["category"]}, skipping mappers', 'error') - return {'created': 0, 'failed': 0} + return {'created': 0, 'updated': 0, 'failed': 0} category = config['category'] org_mappers = config.get('org_mappers', []) team_mappers = config.get('team_mappers', []) + all_new_mappers = org_mappers + team_mappers - total_mappers = len(org_mappers) + len(team_mappers) - if total_mappers == 0: - self._write_output(f'No mappers to create for {category} authenticator') - return {'created': 0, 'failed': 0} + if len(all_new_mappers) == 0: + self._write_output(f'No mappers to process for {category} authenticator') + return {'created': 0, 'updated': 0, 'failed': 0} - self._write_output(f'\n--- Creating mappers for {category} authenticator (ID: {authenticator_id}) ---') + self._write_output(f'\n--- Processing mappers for {category} authenticator (ID: {authenticator_id}) ---') self._write_output(f'Organization mappers: {len(org_mappers)}') self._write_output(f'Team mappers: {len(team_mappers)}') + # Get existing mappers from Gateway + try: + existing_mappers = self.gateway_client.get_authenticator_maps(authenticator_id) + except GatewayAPIError as e: + self._write_output(f'Failed to retrieve existing mappers: {e.message}', 'error') + return {'created': 0, 'updated': 0, 'failed': len(all_new_mappers)} + + # Define mapper-specific ignore keys (can be overridden by subclasses) + ignore_keys = self._get_mapper_ignore_keys() + + # Compare existing vs new mappers + mappers_to_update, mappers_to_create = self._compare_mapper_lists(existing_mappers, all_new_mappers, ignore_keys) + created_count = 0 + updated_count = 0 failed_count = 0 - # Create organization mappers - for mapper in org_mappers: - if self._create_single_mapper(authenticator_id, mapper, 'organization'): - created_count += 1 + # Process updates + for existing_mapper, new_mapper in mappers_to_update: + if self._update_single_mapper(existing_mapper, new_mapper): + updated_count += 1 else: failed_count += 1 - # Create team mappers - for mapper in team_mappers: - if self._create_single_mapper(authenticator_id, mapper, 'team'): + # Process creations + for new_mapper in mappers_to_create: + mapper_type = new_mapper.get('map_type', 'unknown') + if self._create_single_mapper(authenticator_id, new_mapper, mapper_type): created_count += 1 else: failed_count += 1 # Summary - self._write_output(f'Mappers created: {created_count}, failed: {failed_count}') - return {'created': created_count, 'failed': failed_count} + self._write_output(f'Mappers created: {created_count}, updated: {updated_count}, failed: {failed_count}') + return {'created': created_count, 'updated': updated_count, 'failed': failed_count} + + def _get_mapper_ignore_keys(self): + """ + Get list of mapper keys to ignore during comparison. + Can be overridden by subclasses for mapper-specific ignore keys. + + Returns: + list: List of keys to ignore (e.g., auto-generated fields) + """ + return ['id', 'authenticator', 'created', 'modified', 'summary_fields', 'modified_by', 'created_by', 'related', 'url'] + + def _update_single_mapper(self, existing_mapper, new_mapper): + """Update a single mapper in Gateway. + + Args: + existing_mapper: Existing mapper data from Gateway + new_mapper: New mapper configuration to update to + + Returns: + bool: True if mapper was updated successfully, False otherwise + """ + try: + mapper_id = existing_mapper.get('id') + if not mapper_id: + self._write_output(' ✗ Existing mapper missing ID, cannot update', 'error') + return False + + # Prepare update config - don't include fields that shouldn't be updated + update_config = new_mapper.copy() + + # Remove fields that shouldn't be updated (read-only or auto-generated) + fields_to_remove = ['id', 'authenticator', 'created', 'modified'] + for field in fields_to_remove: + update_config.pop(field, None) + + # Update the mapper + self.gateway_client.update_authenticator_map(mapper_id, update_config) + + mapper_name = new_mapper.get('name', 'Unknown') + self._write_output(f' ✓ Updated mapper: {mapper_name}', 'success') + return True + + except GatewayAPIError as e: + mapper_name = new_mapper.get('name', 'Unknown') + self._write_output(f' ✗ Failed to update mapper "{mapper_name}": {e.message}', 'error') + if e.response_data: + self._write_output(f' Details: {e.response_data}', 'error') + return False + except Exception as e: + mapper_name = new_mapper.get('name', 'Unknown') + self._write_output(f' ✗ Unexpected error updating mapper "{mapper_name}": {str(e)}', 'error') + return False def _create_single_mapper(self, authenticator_id, mapper_config, mapper_type): """Create a single mapper in Gateway.""" diff --git a/awx/sso/utils/github_migrator.py b/awx/sso/utils/github_migrator.py index 3f6dd43ece..fd2aef429a 100644 --- a/awx/sso/utils/github_migrator.py +++ b/awx/sso/utils/github_migrator.py @@ -7,7 +7,6 @@ This module handles the migration of GitHub authenticators from AWX to Gateway. from django.conf import settings from awx.conf import settings_registry from awx.main.utils.gateway_mapping import org_map_to_gateway_format, team_map_to_gateway_format -from awx.main.utils.gateway_client import GatewayAPIError from awx.sso.utils.base_migrator import BaseAuthenticatorMigrator import re @@ -140,62 +139,29 @@ class GitHubMigrator(BaseAuthenticatorMigrator): self._write_output(f'Client ID: {key_value}') self._write_output(f'Client Secret: {"*" * 8}') - try: - # Check if authenticator already exists by slug - existing_authenticators = self.gateway_client.get_authenticators() - existing_authenticator = None + # Build Gateway authenticator configuration + gateway_config = { + "name": authenticator_name, + "slug": authenticator_slug, + "type": authenticator_type, + "enabled": True, + "create_objects": True, # Allow Gateway to create users/orgs/teams + "remove_users": False, # Don't remove users by default + "configuration": {"KEY": key_value, "SECRET": secret_value}, + } - for auth in existing_authenticators: - if auth.get('slug') == authenticator_slug: - existing_authenticator = auth - break + # Add any additional configuration based on AWX settings + additional_config = self._build_additional_config(category, settings) + gateway_config["configuration"].update(additional_config) - if existing_authenticator: - # Authenticator already exists, use it - authenticator_id = existing_authenticator.get('id') - self._write_output(f'⚠ Authenticator already exists with ID: {authenticator_id}', 'warning') + # GitHub authenticators have auto-generated fields that should be ignored during comparison + # CALLBACK_URL - automatically created by Gateway + # SCOPE - relevant for mappers with team/org requirement, allows to read the org or team + # SECRET - the secret is encrypted in Gateway, we have no way of comparing the decrypted value + ignore_keys = ['CALLBACK_URL', 'SCOPE', 'SECRET'] - # Store the existing result for mapper creation - config['gateway_authenticator_id'] = authenticator_id - config['gateway_authenticator'] = existing_authenticator - return True - else: - # Authenticator doesn't exist, create it - self._write_output('Creating new authenticator...') - - # Build Gateway authenticator configuration - gateway_config = { - "name": authenticator_name, - "slug": authenticator_slug, - "type": authenticator_type, - "enabled": True, - "create_objects": True, # Allow Gateway to create users/orgs/teams - "remove_users": False, # Don't remove users by default - "configuration": {"KEY": key_value, "SECRET": secret_value}, - } - - # Add any additional configuration based on AWX settings - additional_config = self._build_additional_config(category, settings) - gateway_config["configuration"].update(additional_config) - - # Create the authenticator - result = self.gateway_client.create_authenticator(gateway_config) - - self._write_output(f'✓ Successfully created authenticator with ID: {result.get("id")}', 'success') - - # Store the result for potential mapper creation later - config['gateway_authenticator_id'] = result.get('id') - config['gateway_authenticator'] = result - return True - - except GatewayAPIError as e: - self._write_output(f'✗ Failed to create {category} authenticator: {e.message}', 'error') - if e.response_data: - self._write_output(f' Details: {e.response_data}', 'error') - return False - except Exception as e: - self._write_output(f'✗ Unexpected error creating {category} authenticator: {str(e)}', 'error') - return False + # Submit the authenticator (create or update as needed) + return self.submit_authenticator(gateway_config, ignore_keys, config) def _build_additional_config(self, category, settings): """Build additional configuration for specific authenticator types.""" @@ -210,10 +176,10 @@ class GitHubMigrator(BaseAuthenticatorMigrator): # Add GitHub Enterprise URL if present if 'enterprise' in category: for setting_name, value in settings.items(): - if setting_name.endswith('_URL') and value: - additional_config['URL'] = value - elif setting_name.endswith('_API_URL') and value: + if setting_name.endswith('_API_URL') and value: additional_config['API_URL'] = value + elif setting_name.endswith('_URL') and value: + additional_config['URL'] = value # Add organization name for org-specific authenticators if 'org' in category: