From 44c53b02aebda75541bd01f6e71a20608ef658ef Mon Sep 17 00:00:00 2001 From: Peter Braun Date: Mon, 28 Jul 2025 22:15:59 +0200 Subject: [PATCH] Aap 49709 - settings migration (#7023) * migrate settings using the existing authenticator framework * add method to get settings value to gateway client * add transformer functions for settings * Switched back to PUT for settings updates * Started wiring in testing changes * Added settings_* aggregation results. Added skip-github option. Added tests. Assisted-by: Cursor * Added --skip-all-authenticators command line argument. Added GoogleOAuth testing. Added tests for skipping all authenticators. Assisted-by: Cursor * wip: migrate other missing settings * update login_redirect_override in google_oauth2 * impement login redirect for azuread * implement login redirect for github * implement login redirect for saml * set LOGIN_REDIRECT_OVERRIDE even if no authenticator matched * extract logic for login redirect override to base class * use urlparse to compare valid redirect urls * Preserve the original query parameters * Fix flake8 issues * Preserve the query parameter in sso_login_url Gateway sets the sso_login_url to /api/gateway/social/login/aap-saml-keycloak/?idp=IdP The idp needs to be preserved when creating the redirect * Update awx/main/utils/gateway_client.py Co-authored-by: Chris Meyers * Update awx/main/management/commands/import_auth_config_to_gateway.py Co-authored-by: Chris Meyers * list of settings updated * Update awx/main/utils/gateway_client.py Co-authored-by: Chris Meyers * Update awx/sso/utils/base_migrator.py Co-authored-by: Chris Meyers * fix tests --------- Co-authored-by: Andrew Potozniak Co-authored-by: Madhu Kanoor Co-authored-by: Chris Meyers --- .../commands/import_auth_config_to_gateway.py | 92 +++-- .../test_import_auth_config_to_gateway.py | 124 +++++- .../tests/unit/utils/test_base_migrator.py | 368 ++++++++++++++++++ awx/main/utils/gateway_client.py | 87 +++++ awx/sso/utils/azure_ad_migrator.py | 10 +- awx/sso/utils/base_migrator.py | 105 +++++ awx/sso/utils/github_migrator.py | 19 +- awx/sso/utils/google_oauth2_migrator.py | 11 +- awx/sso/utils/saml_migrator.py | 12 +- awx/sso/utils/settings_migrator.py | 175 +++++++++ 10 files changed, 960 insertions(+), 43 deletions(-) create mode 100644 awx/sso/utils/settings_migrator.py 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 31f232d567..297347b1eb 100644 --- a/awx/main/management/commands/import_auth_config_to_gateway.py +++ b/awx/main/management/commands/import_auth_config_to_gateway.py @@ -9,6 +9,7 @@ from awx.sso.utils.ldap_migrator import LDAPMigrator from awx.sso.utils.oidc_migrator import OIDCMigrator from awx.sso.utils.saml_migrator import SAMLMigrator from awx.sso.utils.radius_migrator import RADIUSMigrator +from awx.sso.utils.settings_migrator import SettingsMigrator from awx.sso.utils.tacacs_migrator import TACACSMigrator from awx.sso.utils.google_oauth2_migrator import GoogleOAuth2Migrator from awx.main.utils.gateway_client import GatewayClient, GatewayAPIError @@ -21,14 +22,25 @@ class Command(BaseCommand): def add_arguments(self, parser): parser.add_argument('--basic-auth', action='store_true', help='Use HTTP Basic Authentication between Controller and Gateway') - parser.add_argument('--skip-oidc', action='store_true', help='Skip importing GitHub and generic OIDC authenticators') + parser.add_argument( + '--skip-all-authenticators', + action='store_true', + help='Skip importing all authenticators [GitHub, OIDC, SAML, Azure AD, LDAP, RADIUS, TACACS+, Google OAuth2]', + ) + parser.add_argument('--skip-oidc', action='store_true', help='Skip importing generic OIDC authenticators') + parser.add_argument('--skip-github', action='store_true', help='Skip importing GitHub authenticator') parser.add_argument('--skip-ldap', action='store_true', help='Skip importing LDAP authenticators') parser.add_argument('--skip-ad', action='store_true', help='Skip importing Azure AD authenticator') parser.add_argument('--skip-saml', action='store_true', help='Skip importing SAML authenticator') parser.add_argument('--skip-radius', action='store_true', help='Skip importing RADIUS authenticator') parser.add_argument('--skip-tacacs', action='store_true', help='Skip importing TACACS+ authenticator') parser.add_argument('--skip-google', action='store_true', help='Skip importing Google OAuth2 authenticator') - parser.add_argument('--force', action='store_true', help='Force migration even if configurations already exist') + parser.add_argument('--skip-settings', action='store_true', help='Skip importing settings') + parser.add_argument( + '--force', + action='store_true', + help='Force migration even if configurations already exist. Does not apply to skipped authenticators nor skipped settings.', + ) def handle(self, *args, **options): # Read Gateway connection parameters from environment variables @@ -37,13 +49,16 @@ class Command(BaseCommand): gateway_password = os.getenv('GATEWAY_PASSWORD') gateway_skip_verify = os.getenv('GATEWAY_SKIP_VERIFY', '').lower() in ('true', '1', 'yes', 'on') + skip_all_authenticators = options['skip_all_authenticators'] skip_oidc = options['skip_oidc'] + skip_github = options['skip_github'] skip_ldap = options['skip_ldap'] skip_ad = options['skip_ad'] skip_saml = options['skip_saml'] skip_radius = options['skip_radius'] skip_tacacs = options['skip_tacacs'] skip_google = options['skip_google'] + skip_settings = options['skip_settings'] force = options['force'] basic_auth = options['basic_auth'] @@ -107,27 +122,38 @@ class Command(BaseCommand): # Initialize migrators migrators = [] - if not skip_oidc: - migrators.append(GitHubMigrator(gateway_client, self, force=force)) - migrators.append(OIDCMigrator(gateway_client, self, force=force)) + if not skip_all_authenticators: + if not skip_oidc: + migrators.append(OIDCMigrator(gateway_client, self, force=force)) - if not skip_saml: - migrators.append(SAMLMigrator(gateway_client, self, force=force)) + if not skip_github: + migrators.append(GitHubMigrator(gateway_client, self, force=force)) - if not skip_ad: - migrators.append(AzureADMigrator(gateway_client, self, force=force)) + if not skip_saml: + migrators.append(SAMLMigrator(gateway_client, self, force=force)) - if not skip_ldap: - migrators.append(LDAPMigrator(gateway_client, self, force=force)) + if not skip_ad: + migrators.append(AzureADMigrator(gateway_client, self, force=force)) - if not skip_radius: - migrators.append(RADIUSMigrator(gateway_client, self, force=force)) + if not skip_ldap: + migrators.append(LDAPMigrator(gateway_client, self, force=force)) - if not skip_tacacs: - migrators.append(TACACSMigrator(gateway_client, self, force=force)) + if not skip_radius: + migrators.append(RADIUSMigrator(gateway_client, self, force=force)) - if not skip_google: - migrators.append(GoogleOAuth2Migrator(gateway_client, self, force=force)) + if not skip_tacacs: + migrators.append(TACACSMigrator(gateway_client, self, force=force)) + + if not skip_google: + migrators.append(GoogleOAuth2Migrator(gateway_client, self, force=force)) + + if not migrators: + self.stdout.write(self.style.WARNING('No authentication configurations found to migrate.')) + + if not skip_settings: + migrators.append(SettingsMigrator(gateway_client, self, force=force)) + else: + self.stdout.write(self.style.WARNING('Settings migration will not execute.')) # Run migrations total_results = { @@ -138,10 +164,14 @@ class Command(BaseCommand): 'mappers_created': 0, 'mappers_updated': 0, 'mappers_failed': 0, + 'settings_created': 0, + 'settings_updated': 0, + 'settings_unchanged': 0, + 'settings_failed': 0, } if not migrators: - self.stdout.write(self.style.WARNING('No authentication configurations found to migrate.')) + self.stdout.write(self.style.WARNING('NO MIGRATIONS WILL EXECUTE.')) else: for migrator in migrators: self.stdout.write(self.style.SUCCESS(f'\n=== Migrating {migrator.get_authenticator_type()} Configurations ===')) @@ -161,6 +191,10 @@ class Command(BaseCommand): 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"]}') + self.stdout.write(f'Total settings created: {total_results["settings_created"]}') + self.stdout.write(f'Total settings updated: {total_results["settings_updated"]}') + self.stdout.write(f'Total settings unchanged: {total_results["settings_unchanged"]}') + self.stdout.write(f'Total settings failed: {total_results["settings_failed"]}') except GatewayAPIError as e: self.stdout.write(self.style.ERROR(f'Gateway API Error: {e.message}')) @@ -176,10 +210,18 @@ 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.get("created", 0)}') - self.stdout.write(f'Authenticators updated: {result.get("updated", 0)}') - self.stdout.write(f'Authenticators unchanged: {result.get("unchanged", 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)}') + + if config_type in ['GitHub', 'OIDC', 'SAML', 'Azure AD', 'LDAP', 'RADIUS', 'TACACS+', 'Google OAuth2']: + self.stdout.write(f'Authenticators created: {result.get("created", 0)}') + self.stdout.write(f'Authenticators updated: {result.get("updated", 0)}') + self.stdout.write(f'Authenticators unchanged: {result.get("unchanged", 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)}') + + if config_type == 'Settings': + self.stdout.write(f'Settings created: {result.get("settings_created", 0)}') + self.stdout.write(f'Settings updated: {result.get("settings_updated", 0)}') + self.stdout.write(f'Settings unchanged: {result.get("settings_unchanged", 0)}') + self.stdout.write(f'Settings failed: {result.get("settings_failed", 0)}') diff --git a/awx/main/tests/unit/commands/test_import_auth_config_to_gateway.py b/awx/main/tests/unit/commands/test_import_auth_config_to_gateway.py index 236c042645..23122610b7 100644 --- a/awx/main/tests/unit/commands/test_import_auth_config_to_gateway.py +++ b/awx/main/tests/unit/commands/test_import_auth_config_to_gateway.py @@ -14,26 +14,32 @@ class TestImportAuthConfigToGatewayCommand(TestCase): def options_basic_auth_full_send(self): return { 'basic_auth': True, + 'skip_all_authenticators': False, 'skip_oidc': False, + 'skip_github': False, 'skip_ldap': False, 'skip_ad': False, 'skip_saml': False, 'skip_radius': False, 'skip_tacacs': False, 'skip_google': False, + 'skip_settings': False, 'force': False, } - def options_basic_auth_skip_all(self): + def options_basic_auth_skip_all_individual(self): return { 'basic_auth': True, + 'skip_all_authenticators': False, 'skip_oidc': True, + 'skip_github': True, 'skip_ldap': True, 'skip_ad': True, 'skip_saml': True, 'skip_radius': True, 'skip_tacacs': True, 'skip_google': True, + 'skip_settings': True, 'force': False, } @@ -43,7 +49,7 @@ class TestImportAuthConfigToGatewayCommand(TestCase): return options def options_svc_token_skip_all(self): - options = self.options_basic_auth_skip_all() + options = self.options_basic_auth_skip_all_individual() options['basic_auth'] = False return options @@ -58,6 +64,10 @@ class TestImportAuthConfigToGatewayCommand(TestCase): mappers_created=0, mappers_updated=0, mappers_failed=0, + settings_created=0, + settings_updated=0, + settings_unchanged=0, + settings_failed=0, ): """Helper method to create a mock migrator with specified return values.""" mock_migrator = Mock() @@ -81,14 +91,25 @@ class TestImportAuthConfigToGatewayCommand(TestCase): expected_calls = [ call('--basic-auth', action='store_true', help='Use HTTP Basic Authentication between Controller and Gateway'), - call('--skip-oidc', action='store_true', help='Skip importing GitHub and generic OIDC authenticators'), + call( + '--skip-all-authenticators', + action='store_true', + help='Skip importing all authenticators [GitHub, OIDC, SAML, Azure AD, LDAP, RADIUS, TACACS+, Google OAuth2]', + ), + call('--skip-oidc', action='store_true', help='Skip importing generic OIDC authenticators'), + call('--skip-github', action='store_true', help='Skip importing GitHub authenticator'), call('--skip-ldap', action='store_true', help='Skip importing LDAP authenticators'), call('--skip-ad', action='store_true', help='Skip importing Azure AD authenticator'), call('--skip-saml', action='store_true', help='Skip importing SAML authenticator'), call('--skip-radius', action='store_true', help='Skip importing RADIUS authenticator'), call('--skip-tacacs', action='store_true', help='Skip importing TACACS+ authenticator'), call('--skip-google', action='store_true', help='Skip importing Google OAuth2 authenticator'), - call('--force', action='store_true', help='Force migration even if configurations already exist'), + call('--skip-settings', action='store_true', help='Skip importing settings'), + call( + '--force', + action='store_true', + help='Force migration even if configurations already exist. Does not apply to skipped authenticators nor skipped settings.', + ), ] parser.add_argument.assert_has_calls(expected_calls, any_order=True) @@ -113,6 +134,7 @@ class TestImportAuthConfigToGatewayCommand(TestCase): os.environ, {'GATEWAY_BASE_URL': 'https://gateway.example.com', 'GATEWAY_USER': 'testuser', 'GATEWAY_PASSWORD': 'testpass', 'GATEWAY_SKIP_VERIFY': 'true'}, ) + @patch('awx.main.management.commands.import_auth_config_to_gateway.SettingsMigrator') @patch.multiple( 'awx.main.management.commands.import_auth_config_to_gateway', GitHubMigrator=DEFAULT, @@ -122,10 +144,11 @@ class TestImportAuthConfigToGatewayCommand(TestCase): LDAPMigrator=DEFAULT, RADIUSMigrator=DEFAULT, TACACSMigrator=DEFAULT, + GoogleOAuth2Migrator=DEFAULT, ) @patch('awx.main.management.commands.import_auth_config_to_gateway.GatewayClient') @patch('sys.stdout', new_callable=StringIO) - def test_handle_basic_auth_success(self, mock_stdout, mock_gateway_client, **mock_migrators): + def test_handle_basic_auth_success(self, mock_stdout, mock_gateway_client, mock_settings_migrator, **mock_migrators): """Test successful execution with basic auth.""" # Mock gateway client context manager mock_client_instance = Mock() @@ -135,6 +158,8 @@ class TestImportAuthConfigToGatewayCommand(TestCase): for mock_migrator_class in mock_migrators.values(): self.create_mock_migrator(mock_migrator_class, created=1, mappers_created=2) + self.create_mock_migrator(mock_settings_migrator, settings_created=1, settings_updated=0, settings_unchanged=2, settings_failed=0) + with patch.object(self.command, 'stdout', mock_stdout): self.command.handle(**self.options_basic_auth_full_send()) @@ -147,12 +172,17 @@ class TestImportAuthConfigToGatewayCommand(TestCase): for mock_migrator in mock_migrators.values(): mock_migrator.assert_called_once_with(mock_client_instance, self.command, force=False) + mock_settings_migrator.assert_called_once_with(mock_client_instance, self.command, force=False) + # Verify output contains success messages output = mock_stdout.getvalue() self.assertIn('HTTP Basic Auth: true', output) self.assertIn('Successfully connected to Gateway', output) self.assertIn('Migration Summary', output) + self.assertIn('authenticators', output) + self.assertIn('mappers', output) + self.assertIn('settings', output) @patch('awx.main.management.commands.import_auth_config_to_gateway.create_api_client') @patch('awx.main.management.commands.import_auth_config_to_gateway.GatewayClientSVCToken') @@ -211,10 +241,12 @@ class TestImportAuthConfigToGatewayCommand(TestCase): LDAPMigrator=DEFAULT, RADIUSMigrator=DEFAULT, TACACSMigrator=DEFAULT, + GoogleOAuth2Migrator=DEFAULT, + SettingsMigrator=DEFAULT, ) @patch('awx.main.management.commands.import_auth_config_to_gateway.GatewayClient') @patch('sys.stdout', new_callable=StringIO) - def test_skip_flags_prevent_migrator_creation(self, mock_stdout, mock_gateway_client, **mock_migrators): + def test_skip_flags_prevent_authenticator_individual_and_settings_migration(self, mock_stdout, mock_gateway_client, **mock_migrators): """Test that skip flags prevent corresponding migrators from being created.""" # Mock gateway client context manager @@ -223,7 +255,7 @@ class TestImportAuthConfigToGatewayCommand(TestCase): mock_gateway_client.return_value.__exit__.return_value = None with patch.object(self.command, 'stdout', mock_stdout): - self.command.handle(**self.options_basic_auth_skip_all()) + self.command.handle(**self.options_basic_auth_skip_all_individual()) # Verify no migrators were created for mock_migrator in mock_migrators.values(): @@ -232,6 +264,46 @@ class TestImportAuthConfigToGatewayCommand(TestCase): # Verify warning message about no configurations output = mock_stdout.getvalue() self.assertIn('No authentication configurations found to migrate.', output) + self.assertIn('Settings migration will not execute.', output) + self.assertIn('NO MIGRATIONS WILL EXECUTE.', output) + + @patch.dict(os.environ, {'GATEWAY_BASE_URL': 'https://gateway.example.com', 'GATEWAY_USER': 'testuser', 'GATEWAY_PASSWORD': 'testpass'}) + @patch.multiple( + 'awx.main.management.commands.import_auth_config_to_gateway', + GitHubMigrator=DEFAULT, + OIDCMigrator=DEFAULT, + SAMLMigrator=DEFAULT, + AzureADMigrator=DEFAULT, + LDAPMigrator=DEFAULT, + RADIUSMigrator=DEFAULT, + TACACSMigrator=DEFAULT, + GoogleOAuth2Migrator=DEFAULT, + ) + @patch('awx.main.management.commands.import_auth_config_to_gateway.GatewayClient') + @patch('sys.stdout', new_callable=StringIO) + def test_skip_flags_prevent_authenticator_migration(self, mock_stdout, mock_gateway_client, **mock_migrators): + """Test that skip flags prevent corresponding migrators from being created.""" + + # Mock gateway client context manager + mock_client_instance = Mock() + mock_gateway_client.return_value.__enter__.return_value = mock_client_instance + mock_gateway_client.return_value.__exit__.return_value = None + + options = self.options_basic_auth_full_send() + options['skip_all_authenticators'] = True + + with patch.object(self.command, 'stdout', mock_stdout): + self.command.handle(**options) + + # Verify no migrators were created + for mock_migrator in mock_migrators.values(): + mock_migrator.assert_not_called() + + # Verify warning message about no configurations + output = mock_stdout.getvalue() + self.assertIn('No authentication configurations found to migrate.', output) + self.assertNotIn('Settings migration will not execute.', output) + self.assertNotIn('NO MIGRATIONS WILL EXECUTE.', output) @patch.dict(os.environ, {'GATEWAY_BASE_URL': 'https://gateway.example.com', 'GATEWAY_USER': 'testuser', 'GATEWAY_PASSWORD': 'testpass'}) @patch('awx.main.management.commands.import_auth_config_to_gateway.GatewayClient') @@ -268,8 +340,9 @@ class TestImportAuthConfigToGatewayCommand(TestCase): @patch.dict(os.environ, {'GATEWAY_BASE_URL': 'https://gateway.example.com', 'GATEWAY_USER': 'testuser', 'GATEWAY_PASSWORD': 'testpass'}) @patch('awx.main.management.commands.import_auth_config_to_gateway.GatewayClient') @patch('awx.main.management.commands.import_auth_config_to_gateway.GitHubMigrator') + @patch('awx.main.management.commands.import_auth_config_to_gateway.SettingsMigrator') @patch('sys.stdout', new_callable=StringIO) - def test_force_flag_passed_to_migrators(self, mock_stdout, mock_github, mock_gateway_client): + def test_force_flag_passed_to_migrators(self, mock_stdout, mock_github, mock_settings_migrator, mock_gateway_client): """Test that force flag is properly passed to migrators.""" # Mock gateway client context manager mock_client_instance = Mock() @@ -278,10 +351,14 @@ class TestImportAuthConfigToGatewayCommand(TestCase): # Mock migrator self.create_mock_migrator(mock_github, authenticator_type="GitHub", created=0, mappers_created=2) + self.create_mock_migrator( + mock_settings_migrator, authenticator_type="Settings", settings_created=0, settings_updated=2, settings_unchanged=0, settings_failed=0 + ) - options = self.options_basic_auth_skip_all() + options = self.options_basic_auth_skip_all_individual() options['force'] = True - options['skip_oidc'] = False + options['skip_github'] = False + options['skip_settings'] = False with patch.object(self.command, 'stdout', mock_stdout): self.command.handle(**options) @@ -289,6 +366,9 @@ class TestImportAuthConfigToGatewayCommand(TestCase): # Verify migrator was created with force=True mock_github.assert_called_once_with(mock_client_instance, self.command, force=True) + # Verify settings migrator was created with force=True + mock_settings_migrator.assert_called_once_with(mock_client_instance, self.command, force=True) + @patch('sys.stdout', new_callable=StringIO) def test_print_export_summary(self, mock_stdout): """Test the _print_export_summary method.""" @@ -315,6 +395,26 @@ class TestImportAuthConfigToGatewayCommand(TestCase): self.assertIn('Mappers updated: 2', output) self.assertIn('Mappers failed: 1', output) + @patch('sys.stdout', new_callable=StringIO) + def test_print_export_summary_settings(self, mock_stdout): + """Test the _print_export_summary method.""" + result = { + 'settings_created': 2, + 'settings_updated': 1, + 'settings_unchanged': 3, + 'settings_failed': 0, + } + + with patch.object(self.command, 'stdout', mock_stdout): + self.command._print_export_summary('Settings', result) + + output = mock_stdout.getvalue() + self.assertIn('--- Settings Export Summary ---', output) + self.assertIn('Settings created: 2', output) + self.assertIn('Settings updated: 1', output) + self.assertIn('Settings unchanged: 3', output) + self.assertIn('Settings failed: 0', output) + @patch('sys.stdout', new_callable=StringIO) def test_print_export_summary_missing_keys(self, mock_stdout): """Test _print_export_summary handles missing keys gracefully.""" @@ -350,7 +450,7 @@ class TestImportAuthConfigToGatewayCommand(TestCase): self.create_mock_migrator(mock_github, authenticator_type="GitHub", created=1, mappers_created=2) self.create_mock_migrator(mock_oidc, authenticator_type="OIDC", created=0, updated=1, unchanged=1, mappers_created=1, mappers_updated=1) - options = self.options_basic_auth_skip_all() + options = self.options_basic_auth_skip_all_individual() options['skip_oidc'] = False options['skip_github'] = False @@ -401,7 +501,7 @@ class TestImportAuthConfigToGatewayCommand(TestCase): mock_gateway_client.return_value.__exit__.return_value = None with patch.object(self.command, 'stdout', mock_stdout): - self.command.handle(**self.options_basic_auth_skip_all()) + self.command.handle(**self.options_basic_auth_skip_all_individual()) # Verify gateway client was called with correct skip_verify value mock_gateway_client.assert_called_once_with( diff --git a/awx/main/tests/unit/utils/test_base_migrator.py b/awx/main/tests/unit/utils/test_base_migrator.py index d6713cd8b5..0af8fdf1d2 100644 --- a/awx/main/tests/unit/utils/test_base_migrator.py +++ b/awx/main/tests/unit/utils/test_base_migrator.py @@ -869,3 +869,371 @@ class TestSocialAuthMapFunctions: result = self.migrator.get_social_team_map(setting_name) assert result == {'test_team': {'organization': 'test_org'}} + + +class TestHandleLoginOverride: + """Tests for handle_login_override method.""" + + def setup_method(self): + """Set up test fixtures.""" + self.gateway_client = Mock() + self.command = Mock() + self.migrator = BaseAuthenticatorMigrator(self.gateway_client, self.command) + + # Reset the class-level flag before each test + BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator = False + + def test_handle_login_override_no_login_redirect_override(self): + """Test that method returns early when no login_redirect_override is provided.""" + config = {} + valid_login_urls = ['/sso/login/github'] + + self.migrator.handle_login_override(config, valid_login_urls) + + # Should not call any gateway client methods + self.gateway_client.get_base_url.assert_not_called() + self.gateway_client.update_gateway_setting.assert_not_called() + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is False + + def test_handle_login_override_empty_login_redirect_override(self): + """Test that method returns early when login_redirect_override is empty.""" + config = {'login_redirect_override': ''} + valid_login_urls = ['/sso/login/github'] + + self.migrator.handle_login_override(config, valid_login_urls) + + # Should not call any gateway client methods + self.gateway_client.get_base_url.assert_not_called() + self.gateway_client.update_gateway_setting.assert_not_called() + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is False + + def test_handle_login_override_no_url_match(self): + """Test that method returns early when login_redirect_override doesn't match valid URLs.""" + config = {'login_redirect_override': 'https://localhost:3000/sso/login/saml'} + valid_login_urls = ['/sso/login/github', '/sso/login/azuread-oauth2'] + + self.migrator.handle_login_override(config, valid_login_urls) + + # Should not call any gateway client methods + self.gateway_client.get_base_url.assert_not_called() + self.gateway_client.update_gateway_setting.assert_not_called() + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is False + + def test_handle_login_override_no_gateway_authenticator(self): + """Test that method returns early when gateway_authenticator is missing.""" + config = {'login_redirect_override': 'https://localhost:3000/sso/login/github'} + valid_login_urls = ['/sso/login/github'] + + self.migrator.handle_login_override(config, valid_login_urls) + + # Should not call any gateway client methods + self.gateway_client.get_base_url.assert_not_called() + self.gateway_client.update_gateway_setting.assert_not_called() + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is False + + def test_handle_login_override_empty_gateway_authenticator(self): + """Test that method returns early when gateway_authenticator is empty.""" + config = {'login_redirect_override': 'https://localhost:3000/sso/login/github', 'gateway_authenticator': {}} + valid_login_urls = ['/sso/login/github'] + + self.migrator.handle_login_override(config, valid_login_urls) + + # Should not call any gateway client methods + self.gateway_client.get_base_url.assert_not_called() + self.gateway_client.update_gateway_setting.assert_not_called() + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is False + + def test_handle_login_override_no_sso_login_url(self): + """Test that method returns early when sso_login_url is missing.""" + config = {'login_redirect_override': 'https://localhost:3000/sso/login/github', 'gateway_authenticator': {'id': 123}} + valid_login_urls = ['/sso/login/github'] + + self.migrator.handle_login_override(config, valid_login_urls) + + # Should not call any gateway client methods + self.gateway_client.get_base_url.assert_not_called() + self.gateway_client.update_gateway_setting.assert_not_called() + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is False + + def test_handle_login_override_empty_sso_login_url(self): + """Test that method returns early when sso_login_url is empty.""" + config = {'login_redirect_override': 'https://localhost:3000/sso/login/github', 'gateway_authenticator': {'id': 123, 'sso_login_url': ''}} + valid_login_urls = ['/sso/login/github'] + + self.migrator.handle_login_override(config, valid_login_urls) + + # Should not call any gateway client methods + self.gateway_client.get_base_url.assert_not_called() + self.gateway_client.update_gateway_setting.assert_not_called() + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is False + + def test_handle_login_override_successful_update(self): + """Test successful LOGIN_REDIRECT_OVERRIDE update.""" + config = { + 'login_redirect_override': 'https://localhost:3000/sso/login/github', + 'gateway_authenticator': {'id': 123, 'sso_login_url': '/sso/auth/login/123/'}, + } + valid_login_urls = ['/sso/login/github'] + + # Mock gateway client methods + self.gateway_client.get_base_url.return_value = 'https://gateway.example.com' + self.migrator.handle_login_override(config, valid_login_urls) + + # Verify gateway client methods were called correctly + self.gateway_client.get_base_url.assert_called_once() + self.gateway_client.update_gateway_setting.assert_called_once_with('LOGIN_REDIRECT_OVERRIDE', 'https://gateway.example.com/sso/auth/login/123/') + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is True + + def test_handle_login_override_multiple_valid_urls_first_matches(self): + """Test that first matching URL in valid_login_urls is used.""" + config = { + 'login_redirect_override': 'https://localhost:3000/sso/login/github-org', + 'gateway_authenticator': {'id': 123, 'sso_login_url': '/sso/auth/login/123/'}, + } + valid_login_urls = ['/sso/login/github-org', '/sso/login/github-team', '/sso/login/github'] + + # Mock gateway client methods + self.gateway_client.get_base_url.return_value = 'https://gateway.example.com' + + self.migrator.handle_login_override(config, valid_login_urls) + + # Should still work since first URL matches + self.gateway_client.update_gateway_setting.assert_called_once_with('LOGIN_REDIRECT_OVERRIDE', 'https://gateway.example.com/sso/auth/login/123/') + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is True + + def test_handle_login_override_multiple_valid_urls_last_matches(self): + """Test that last matching URL in valid_login_urls is used.""" + config = { + 'login_redirect_override': 'https://localhost:3000/sso/login/github', + 'gateway_authenticator': {'id': 123, 'sso_login_url': '/sso/auth/login/123/'}, + } + valid_login_urls = ['/sso/login/github-org', '/sso/login/github-team', '/sso/login/github'] + + # Mock gateway client methods + self.gateway_client.get_base_url.return_value = 'https://gateway.example.com' + + self.migrator.handle_login_override(config, valid_login_urls) + + # Should work since last URL matches + self.gateway_client.update_gateway_setting.assert_called_once_with('LOGIN_REDIRECT_OVERRIDE', 'https://gateway.example.com/sso/auth/login/123/') + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is True + + def test_handle_login_override_partial_url_match(self): + """Test that partial URL matching works (using 'in' operator).""" + config = { + 'login_redirect_override': 'https://controller.example.com/sso/login/azuread-oauth2/?next=%2Fdashboard', + 'gateway_authenticator': {'id': 456, 'sso_login_url': '/auth/login/azuread/456/'}, + } + valid_login_urls = ['/sso/login/azuread-oauth2'] + + # Mock gateway client methods + self.gateway_client.get_base_url.return_value = 'https://gateway.example.com:8080' + self.migrator.handle_login_override(config, valid_login_urls) + + # Should work since valid URL is contained in login_redirect_override + self.gateway_client.update_gateway_setting.assert_called_once_with( + 'LOGIN_REDIRECT_OVERRIDE', 'https://gateway.example.com:8080/auth/login/azuread/456/?next=%2Fdashboard' + ) + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is True + + def test_handle_login_override_saml_with_parameters(self): + """Test LOGIN_REDIRECT_OVERRIDE with SAML IDP parameters.""" + config = { + 'login_redirect_override': 'https://localhost:3000/sso/login/saml/?idp=mycompany', + 'gateway_authenticator': {'id': 789, 'sso_login_url': '/auth/login/saml/789/'}, + } + valid_login_urls = ['/sso/login/saml/?idp=mycompany'] + + # Mock gateway client methods + self.gateway_client.get_base_url.return_value = 'https://gateway.local' + + self.migrator.handle_login_override(config, valid_login_urls) + + # Should work with SAML parameter URLs + self.gateway_client.update_gateway_setting.assert_called_once_with( + 'LOGIN_REDIRECT_OVERRIDE', 'https://gateway.local/auth/login/saml/789/?idp=mycompany' + ) + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is True + + def test_handle_login_override_github_with_trailing_slash(self): + """Test LOGIN_REDIRECT_OVERRIDE with trailing slash.""" + config = { + 'login_redirect_override': 'https://localhost:3000/sso/login/github-enterprise/', + 'gateway_authenticator': {'id': 999, 'sso_login_url': '/auth/login/github/999/'}, + } + valid_login_urls = ['/sso/login/github-enterprise', '/sso/login/github-enterprise/'] + + # Mock gateway client methods + self.gateway_client.get_base_url.return_value = 'https://gateway.internal' + + self.migrator.handle_login_override(config, valid_login_urls) + + # Should work with trailing slash URLs + self.gateway_client.update_gateway_setting.assert_called_once_with('LOGIN_REDIRECT_OVERRIDE', 'https://gateway.internal/auth/login/github/999/') + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is True + + def test_handle_login_override_empty_valid_urls_list(self): + """Test that method returns early when valid_login_urls is empty.""" + config = { + 'login_redirect_override': 'https://localhost:3000/sso/login/github', + 'gateway_authenticator': {'id': 123, 'sso_login_url': '/sso/auth/login/123/'}, + } + valid_login_urls = [] + + self.migrator.handle_login_override(config, valid_login_urls) + + # Should not call any gateway client methods + self.gateway_client.get_base_url.assert_not_called() + self.gateway_client.update_gateway_setting.assert_not_called() + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is False + + def test_handle_login_override_preserves_existing_flag_state(self): + """Test that method preserves flag state if it was already set.""" + # Set flag to True initially + BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator = True + + config = { + 'login_redirect_override': 'https://localhost:3000/sso/login/github', + 'gateway_authenticator': {'id': 123, 'sso_login_url': '/sso/auth/login/123/'}, + } + valid_login_urls = ['/sso/login/github'] + + # Mock gateway client methods + self.gateway_client.get_base_url.return_value = 'https://gateway.example.com' + + self.migrator.handle_login_override(config, valid_login_urls) + + # Flag should still be True + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is True + + def test_handle_login_override_writes_output_message(self): + """Test that method writes output message when updating.""" + config = { + 'login_redirect_override': 'https://localhost:3000/sso/login/google-oauth2', + 'gateway_authenticator': {'id': 555, 'sso_login_url': '/auth/login/google/555/'}, + } + valid_login_urls = ['/sso/login/google-oauth2'] + + # Mock gateway client methods + self.gateway_client.get_base_url.return_value = 'https://gateway.test' + + # Mock _write_output method + with patch.object(self.migrator, '_write_output') as mock_write_output: + self.migrator.handle_login_override(config, valid_login_urls) + + # Verify output message was written + mock_write_output.assert_called_once_with('Updating LOGIN_REDIRECT_OVERRIDE to: https://gateway.test/auth/login/google/555/') + + @pytest.mark.parametrize( + "login_redirect_override,valid_urls,expected_match", + [ + # Test Azure AD variations + ('https://localhost:3000/sso/login/azuread-oauth2', ['/sso/login/azuread-oauth2'], True), + ('https://localhost:3000/sso/login/azuread-oauth2/', ['/sso/login/azuread-oauth2'], True), + ('https://controller.example.com/sso/login/azuread-oauth2?next=/home', ['/sso/login/azuread-oauth2'], True), + # Test Google OAuth2 variations + ('https://localhost:3000/sso/login/google-oauth2', ['/sso/login/google-oauth2'], True), + ('https://localhost:3000/sso/login/google-oauth2/', ['/sso/login/google-oauth2'], True), + # Test GitHub variations + ('https://localhost:3000/sso/login/github', ['/sso/login/github'], True), + ('https://localhost:3000/sso/login/github-org', ['/sso/login/github-org'], True), + ('https://localhost:3000/sso/login/github-team', ['/sso/login/github-team'], True), + ('https://localhost:3000/sso/login/github-enterprise', ['/sso/login/github-enterprise'], True), + # Test SAML variations + ('https://localhost:3000/sso/login/saml/?idp=company', ['/sso/login/saml/?idp=company'], True), + ('https://localhost:3000/sso/login/saml/?idp=test-org', ['/sso/login/saml/?idp=test-org'], True), + # Test non-matching cases + ('https://localhost:3000/sso/login/ldap', ['/sso/login/github'], False), + ('https://localhost:3000/sso/login/azuread-oauth2', ['/sso/login/google-oauth2'], False), + ('https://localhost:3000/sso/login/saml/?idp=wrong', ['/sso/login/saml/?idp=company'], False), + # Test multiple valid URLs + ('https://localhost:3000/sso/login/github-org', ['/sso/login/github', '/sso/login/github-org'], True), + ('https://localhost:3000/sso/login/github', ['/sso/login/github-org', '/sso/login/github'], True), + # Test improved URL parsing scenarios - better boundary detection + ('https://localhost:3000/sso/login/github-enterprise', ['/sso/login/github'], False), # Should NOT match due to better parsing + ('https://localhost:3000/sso/login/saml/?idp=company&next=/home', ['/sso/login/saml/?idp=company'], True), + ('https://localhost:3000/sso/login/saml/?idp=company', ['/sso/login/saml/?idp=different'], False), + ('https://controller.example.com:8080/sso/login/azuread-oauth2/?next=/dashboard', ['/sso/login/azuread-oauth2'], True), + ('http://localhost/sso/login/github?state=abc123', ['/sso/login/github'], True), + # Test boundary detection edge cases + ('https://localhost:3000/sso/login/github/', ['/sso/login/github'], True), # Trailing slash should match + ('https://localhost:3000/sso/login/github#section', ['/sso/login/github'], True), # Fragment should match + ], + ) + def test_handle_login_override_url_matching_variations(self, login_redirect_override, valid_urls, expected_match): + """Test various URL matching scenarios parametrically.""" + config = {'login_redirect_override': login_redirect_override, 'gateway_authenticator': {'id': 123, 'sso_login_url': '/auth/login/test/123/'}} + + # Mock gateway client methods + self.gateway_client.get_base_url.return_value = 'https://gateway.test' + + self.migrator.handle_login_override(config, valid_urls) + + if expected_match: + # Should call gateway methods when URL matches + self.gateway_client.get_base_url.assert_called_once() + self.gateway_client.update_gateway_setting.assert_called_once() + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is True + else: + # Should not call gateway methods when URL doesn't match + self.gateway_client.get_base_url.assert_not_called() + self.gateway_client.update_gateway_setting.assert_not_called() + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is False + + def test_handle_login_override_improved_url_parsing(self): + """Test that improved URL parsing with proper path boundary detection prevents false positive matches.""" + # This test demonstrates the improvement over simple string matching + config = { + 'login_redirect_override': 'https://localhost:3000/sso/login/github-enterprise', + 'gateway_authenticator': {'id': 123, 'sso_login_url': '/auth/login/test/123/'}, + } + + # With the old simple string matching, this would incorrectly match + # because '/sso/login/github' is contained in '/sso/login/github-enterprise' + # But with proper URL parsing, it should NOT match + valid_login_urls = ['/sso/login/github'] + + self.migrator.handle_login_override(config, valid_login_urls) + + # Should NOT match due to improved parsing + self.gateway_client.get_base_url.assert_not_called() + self.gateway_client.update_gateway_setting.assert_not_called() + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is False + + def test_handle_login_override_query_parameter_handling(self): + """Test that query parameters are properly handled in URL matching.""" + config = { + 'login_redirect_override': 'https://localhost:3000/sso/login/saml/?idp=mycompany&next=%2Fdashboard', + 'gateway_authenticator': {'id': 456, 'sso_login_url': '/auth/login/saml/456/?idp=IdP'}, + } + + # Should match the SAML URL with the correct IDP parameter (boundary-aware matching) + valid_login_urls = ['/sso/login/saml/?idp=mycompany'] + + self.gateway_client.get_base_url.return_value = 'https://gateway.test' + + self.migrator.handle_login_override(config, valid_login_urls) + + # Should match because the query parameter is properly contained with boundaries + self.gateway_client.get_base_url.assert_called_once() + self.gateway_client.update_gateway_setting.assert_called_once_with( + 'LOGIN_REDIRECT_OVERRIDE', 'https://gateway.test/auth/login/saml/456/?idp=IdP&next=%2Fdashboard' + ) + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is True + + def test_handle_login_override_different_query_parameters(self): + """Test that different query parameters don't match.""" + config = { + 'login_redirect_override': 'https://localhost:3000/sso/login/saml/?idp=company-a', + 'gateway_authenticator': {'id': 456, 'sso_login_url': '/auth/login/saml/456/'}, + } + + # Should NOT match SAML URL with different IDP parameter + valid_login_urls = ['/sso/login/saml/?idp=company-b'] + + self.migrator.handle_login_override(config, valid_login_urls) + + # Should NOT match because the query parameters are different + self.gateway_client.get_base_url.assert_not_called() + self.gateway_client.update_gateway_setting.assert_not_called() + assert BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator is False diff --git a/awx/main/utils/gateway_client.py b/awx/main/utils/gateway_client.py index 748423c21a..8ae72cd36b 100644 --- a/awx/main/utils/gateway_client.py +++ b/awx/main/utils/gateway_client.py @@ -397,6 +397,93 @@ class GatewayClient: return self.create_authenticator(config) + def update_gateway_setting(self, setting_name: str, setting_value: Any) -> Dict[str, Any]: + """Update a Gateway setting via the settings API. + + Args: + setting_name: Name of the setting to update + setting_value: Value to set for the setting + + Returns: + dict: Upon successful update, well formed responses are returned, otherwise the original payload is returned. + + Raises: + GatewayAPIError: If update fails, anything other than a 200 or 204 response code. + """ + endpoint = '/api/gateway/v1/settings/all/' + + # Create the JSON payload with the setting name and value + payload = {setting_name: setting_value} + + try: + response = self._make_request('PUT', endpoint, data=payload) + + if response.status_code in [200, 204]: + logger.info(f"Successfully updated Gateway setting: {setting_name}") + # Return the response data if available, otherwise return the payload + if response.content: + try: + return response.json() + except requests.exceptions.JSONDecodeError: + return payload + return payload + else: + error_msg = f"Failed to update Gateway setting. 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 Gateway setting: {str(e)}") + + def get_gateway_setting(self, setting_name: str) -> Any: + """Get a Gateway setting value via the settings API. + + Args: + setting_name: Name of the setting to retrieve + + Returns: + Any: The value of the setting, or None if not found + + Raises: + GatewayAPIError: If request fails + """ + endpoint = '/api/gateway/v1/settings/all/' + + try: + response = self._make_request('GET', endpoint) + + if response.status_code == 200: + settings_data = response.json() + logger.info("Successfully retrieved Gateway settings") + + # Return the specific setting value or None if not found + return settings_data.get(setting_name) + else: + error_msg = f"Failed to get Gateway settings. 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 get Gateway setting: {str(e)}") + + def get_base_url(self) -> str: + """Get the base URL of the Gateway instance. + + Returns: + str: The base URL of the Gateway instance + """ + return self.base_url + def close(self): """Close the session and clean up resources.""" if self.session: diff --git a/awx/sso/utils/azure_ad_migrator.py b/awx/sso/utils/azure_ad_migrator.py index b17cd3d8b7..723fece89e 100644 --- a/awx/sso/utils/azure_ad_migrator.py +++ b/awx/sso/utils/azure_ad_migrator.py @@ -36,6 +36,7 @@ class AzureADMigrator(BaseAuthenticatorMigrator): # If we have both key and secret, collect all settings org_map_value = getattr(settings, 'SOCIAL_AUTH_AZUREAD_OAUTH2_ORGANIZATION_MAP', None) team_map_value = getattr(settings, 'SOCIAL_AUTH_AZUREAD_OAUTH2_TEAM_MAP', None) + login_redirect_override = getattr(settings, "LOGIN_REDIRECT_OVERRIDE", None) # Convert GitHub org and team mappings from AWX to the Gateway format # Start with order 1 and maintain sequence across both org and team mappers @@ -65,6 +66,7 @@ class AzureADMigrator(BaseAuthenticatorMigrator): }, 'org_mappers': org_mappers, 'team_mappers': team_mappers, + 'login_redirect_override': login_redirect_override, } ] @@ -85,4 +87,10 @@ class AzureADMigrator(BaseAuthenticatorMigrator): ignore_keys = ["CALLBACK_URL", "GROUPS_CLAIM"] # Submit the authenticator (create or update as needed) - return self.submit_authenticator(gateway_config, ignore_keys, config) + result = self.submit_authenticator(gateway_config, ignore_keys, config) + + # Handle LOGIN_REDIRECT_OVERRIDE if applicable + valid_login_urls = ['/sso/login/azuread-oauth2'] + self.handle_login_override(config, valid_login_urls) + + return result diff --git a/awx/sso/utils/base_migrator.py b/awx/sso/utils/base_migrator.py index 50468547c4..0c1c5070fe 100644 --- a/awx/sso/utils/base_migrator.py +++ b/awx/sso/utils/base_migrator.py @@ -4,6 +4,7 @@ Base authenticator migrator class. This module defines the contract that all specific authenticator migrators must follow. """ +from urllib.parse import urlparse, parse_qs, urlencode from django.conf import settings from awx.main.utils.gateway_client import GatewayAPIError @@ -14,6 +15,10 @@ class BaseAuthenticatorMigrator: Defines the contract that all specific authenticator migrators must follow. """ + KEYS_TO_PRESERVE = ['idp'] + # Class-level flag to track if LOGIN_REDIRECT_OVERRIDE was set by any migrator + login_redirect_override_set_by_migrator = False + def __init__(self, gateway_client=None, command=None, force=False): """ Initialize the authenticator migrator. @@ -88,6 +93,7 @@ class BaseAuthenticatorMigrator: mappers_updated += mapper_result['updated'] mappers_failed += mapper_result['failed'] + # Authenticators don't have settings, so settings counts are always 0 return { 'created': len(created_authenticators), 'updated': len(updated_authenticators), @@ -96,6 +102,10 @@ class BaseAuthenticatorMigrator: 'mappers_created': mappers_created, 'mappers_updated': mappers_updated, 'mappers_failed': mappers_failed, + 'settings_created': 0, + 'settings_updated': 0, + 'settings_unchanged': 0, + 'settings_failed': 0, } def get_controller_config(self): @@ -553,6 +563,101 @@ class BaseAuthenticatorMigrator: global_map = getattr(settings, 'SOCIAL_AUTH_TEAM_MAP', {}) return global_map + def handle_login_override(self, config, valid_login_urls): + """ + Handle LOGIN_REDIRECT_OVERRIDE setting for this authenticator. + + This method checks if the login_redirect_override from the config matches + any of the provided valid_login_urls. If it matches, it updates the + LOGIN_REDIRECT_OVERRIDE setting in Gateway with the new authenticator's + URL and sets the class flag to indicate it was handled. + + Args: + config: Configuration dictionary containing: + - login_redirect_override: The current LOGIN_REDIRECT_OVERRIDE value + - gateway_authenticator: The created/updated authenticator info + valid_login_urls: List of URL patterns to match against + """ + login_redirect_override = config.get('login_redirect_override') + if not login_redirect_override: + return + + # Check if the login_redirect_override matches any of the provided valid URLs + url_matches = False + parsed_redirect = urlparse(login_redirect_override) + self.redirect_query_dict = parse_qs(parsed_redirect.query, keep_blank_values=True) if parsed_redirect.query else {} + + for valid_url in valid_login_urls: + parsed_valid = urlparse(valid_url) + + # Compare path: redirect path should match or contain the valid path at proper boundaries + if parsed_redirect.path == parsed_valid.path: + path_matches = True + elif parsed_redirect.path.startswith(parsed_valid.path): + # Ensure the match is at a path boundary (followed by '/' or end of string) + next_char_pos = len(parsed_valid.path) + if next_char_pos >= len(parsed_redirect.path) or parsed_redirect.path[next_char_pos] in ['/', '?']: + path_matches = True + else: + path_matches = False + else: + path_matches = False + + # Compare query: if valid URL has query params, they should be present in redirect URL + query_matches = True + if parsed_valid.query: + # Parse query parameters for both URLs + valid_params = parse_qs(parsed_valid.query, keep_blank_values=True) + + # All valid URL query params must be present in redirect URL with same values + query_matches = all(param in self.redirect_query_dict and self.redirect_query_dict[param] == values for param, values in valid_params.items()) + + if path_matches and query_matches: + url_matches = True + break + + if not url_matches: + return + + # Extract the created authenticator from config + gateway_authenticator = config.get('gateway_authenticator') + if not gateway_authenticator: + return + + sso_login_url = gateway_authenticator.get('sso_login_url') + if not sso_login_url: + return + + # Update LOGIN_REDIRECT_OVERRIDE with the new Gateway URL + gateway_base_url = self.gateway_client.get_base_url() + parsed_sso = urlparse(sso_login_url) + parsed_gw = urlparse(gateway_base_url) + updated_query = self._updated_query_string(parsed_sso) + complete_url = parsed_redirect._replace(scheme=parsed_gw.scheme, path=parsed_sso.path, netloc=parsed_gw.netloc, query=updated_query).geturl() + self._write_output(f'Updating LOGIN_REDIRECT_OVERRIDE to: {complete_url}') + self.gateway_client.update_gateway_setting('LOGIN_REDIRECT_OVERRIDE', complete_url) + + # Set the class-level flag to indicate LOGIN_REDIRECT_OVERRIDE was handled by a migrator + BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator = True + + def _updated_query_string(self, parsed_sso): + if parsed_sso.query: + parsed_sso_dict = parse_qs(parsed_sso.query, keep_blank_values=True) + else: + parsed_sso_dict = {} + + result = {} + for k, v in self.redirect_query_dict.items(): + if k in self.KEYS_TO_PRESERVE and k in parsed_sso_dict: + v = parsed_sso_dict[k] + + if isinstance(v, list) and len(v) == 1: + result[k] = v[0] + else: + result[k] = v + + return urlencode(result, doseq=True) if result else "" + def _write_output(self, message, style=None): """Write output message if command is available.""" if self.command: diff --git a/awx/sso/utils/github_migrator.py b/awx/sso/utils/github_migrator.py index 273e8bb672..acbb3d74a0 100644 --- a/awx/sso/utils/github_migrator.py +++ b/awx/sso/utils/github_migrator.py @@ -29,6 +29,7 @@ class GitHubMigrator(BaseAuthenticatorMigrator): list: List of configured GitHub authentication providers with their settings """ github_categories = ['github', 'github-org', 'github-team', 'github-enterprise', 'github-enterprise-org', 'github-enterprise-team'] + login_redirect_override = getattr(settings, "LOGIN_REDIRECT_OVERRIDE", None) found_configs = [] @@ -91,7 +92,15 @@ class GitHubMigrator(BaseAuthenticatorMigrator): org_mappers, next_order = org_map_to_gateway_format(org_map_value, start_order=1) team_mappers, _ = team_map_to_gateway_format(team_map_value, start_order=next_order) - found_configs.append({'category': category, 'settings': config_data, 'org_mappers': org_mappers, 'team_mappers': team_mappers}) + found_configs.append( + { + 'category': category, + 'settings': config_data, + 'org_mappers': org_mappers, + 'team_mappers': team_mappers, + 'login_redirect_override': login_redirect_override, + } + ) except Exception as e: raise Exception(f'Could not retrieve {category} settings: {str(e)}') @@ -165,7 +174,13 @@ class GitHubMigrator(BaseAuthenticatorMigrator): ignore_keys = ['CALLBACK_URL', 'SCOPE'] # Submit the authenticator (create or update as needed) - return self.submit_authenticator(gateway_config, ignore_keys, config) + result = self.submit_authenticator(gateway_config, ignore_keys, config) + + # Handle LOGIN_REDIRECT_OVERRIDE if applicable + valid_login_urls = [f'/sso/login/{category}', f'/sso/login/{category}/'] + self.handle_login_override(config, valid_login_urls) + + return result def _build_additional_config(self, category, settings): """Build additional configuration for specific authenticator types.""" diff --git a/awx/sso/utils/google_oauth2_migrator.py b/awx/sso/utils/google_oauth2_migrator.py index 8be1914503..4709e02241 100644 --- a/awx/sso/utils/google_oauth2_migrator.py +++ b/awx/sso/utils/google_oauth2_migrator.py @@ -37,10 +37,13 @@ class GoogleOAuth2Migrator(BaseAuthenticatorMigrator): 'SOCIAL_AUTH_GOOGLE_OAUTH2_SCOPE': settings.SOCIAL_AUTH_GOOGLE_OAUTH2_SCOPE, } + login_redirect_override = getattr(settings, "LOGIN_REDIRECT_OVERRIDE", None) + return [ { "category": self.get_authenticator_type(), "settings": config_data, + "login_redirect_override": login_redirect_override, } ] @@ -90,4 +93,10 @@ class GoogleOAuth2Migrator(BaseAuthenticatorMigrator): else: ignore_keys.append(key) - return self.submit_authenticator(gateway_config, ignore_keys, config) + result = self.submit_authenticator(gateway_config, ignore_keys, config) + + # Handle LOGIN_REDIRECT_OVERRIDE if applicable + valid_login_urls = ['/sso/login/google-oauth2'] + self.handle_login_override(config, valid_login_urls) + + return result diff --git a/awx/sso/utils/saml_migrator.py b/awx/sso/utils/saml_migrator.py index 7a4f93dd07..8b481e2d04 100644 --- a/awx/sso/utils/saml_migrator.py +++ b/awx/sso/utils/saml_migrator.py @@ -86,6 +86,7 @@ class SAMLMigrator(BaseAuthenticatorMigrator): saml_team_attr = getattr(settings, "SOCIAL_AUTH_SAML_TEAM_ATTR", {}) org_attr = getattr(settings, "SOCIAL_AUTH_SAML_ORGANIZATION_ATTR", {}) user_flags_by_attr = getattr(settings, "SOCIAL_AUTH_SAML_USER_FLAGS_BY_ATTR", {}) + login_redirect_override = getattr(settings, "LOGIN_REDIRECT_OVERRIDE", None) org_mappers, self.next_order = org_map_to_gateway_format(org_map_value, start_order=self.next_order) self.team_mappers, self.next_order = team_map_to_gateway_format(team_map_value, start_order=self.next_order) @@ -135,6 +136,7 @@ class SAMLMigrator(BaseAuthenticatorMigrator): "settings": config_data, "org_mappers": org_mappers, "team_mappers": self.team_mappers, + "login_redirect_override": login_redirect_override, } ) return found_configs @@ -147,7 +149,7 @@ class SAMLMigrator(BaseAuthenticatorMigrator): # Generate authenticator name and slug authenticator_name = f"AWX-{category.replace('-', '_').title()}-{name}" - authenticator_slug = self._generate_authenticator_slug("saml", category) + authenticator_slug = self._generate_authenticator_slug("saml", name) self._write_output(f"\n--- Processing {category} authenticator ---") self._write_output(f"Name: {authenticator_name}") @@ -169,7 +171,13 @@ class SAMLMigrator(BaseAuthenticatorMigrator): ignore_keys = ["CALLBACK_URL", "SP_PRIVATE_KEY"] # Submit the authenticator (create or update as needed) - return self.submit_authenticator(gateway_config, ignore_keys, config) + result = self.submit_authenticator(gateway_config, ignore_keys, config) + + # Handle LOGIN_REDIRECT_OVERRIDE if applicable + valid_login_urls = [f'/sso/login/saml/?idp={name}', f'/sso/login/saml/?idp={name}/'] + self.handle_login_override(config, valid_login_urls) + + return result def _team_attr_to_gateway_format(self, saml_team_attr): saml_attr = saml_team_attr.get("saml_attr") diff --git a/awx/sso/utils/settings_migrator.py b/awx/sso/utils/settings_migrator.py new file mode 100644 index 0000000000..9b963de900 --- /dev/null +++ b/awx/sso/utils/settings_migrator.py @@ -0,0 +1,175 @@ +""" +Settings migrator. + +This module handles the migration of AWX settings to Gateway. +""" + +from django.conf import settings +from awx.sso.utils.base_migrator import BaseAuthenticatorMigrator + + +class SettingsMigrator(BaseAuthenticatorMigrator): + """ + Handles the migration of AWX settings to Gateway. + """ + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + # Define transformer functions for each setting + self.setting_transformers = { + 'SOCIAL_AUTH_USERNAME_IS_FULL_EMAIL': self._transform_social_auth_username_is_full_email, + 'ALLOW_OAUTH2_FOR_EXTERNAL_USERS': self._transform_allow_oauth2_for_external_users, + } + + def _convert_setting_name(self, setting): + keys = { + "CUSTOM_LOGIN_INFO": "custom_login_info", + "CUSTOM_LOGO": "custom_logo", + } + return keys.get(setting, setting) + + def _transform_social_auth_username_is_full_email(self, value): + # SOCIAL_AUTH_USERNAME_IS_FULL_EMAIL is a boolean and does not need to be transformed + return value + + def _transform_allow_oauth2_for_external_users(self, value): + # ALLOW_OAUTH2_FOR_EXTERNAL_USERS is a boolean and does not need to be transformed + return value + + def get_authenticator_type(self): + """Get the human-readable authenticator type name.""" + return "Settings" + + def get_controller_config(self): + """ + Export relevant AWX settings that need to be migrated to Gateway. + + Returns: + list: List of configured settings that need to be migrated + """ + # Define settings that should be migrated from AWX to Gateway + settings_to_migrate = ['SESSION_COOKIE_AGE', 'SOCIAL_AUTH_USERNAME_IS_FULL_EMAIL', 'ALLOW_OAUTH2_FOR_EXTERNAL_USERS'] + + # Add LOGIN_REDIRECT_OVERRIDE to the list if no authenticator migrator has handled it + if not BaseAuthenticatorMigrator.login_redirect_override_set_by_migrator: + settings_to_migrate.append("LOGIN_REDIRECT_OVERRIDE") + + found_configs = [] + + for setting_name in settings_to_migrate: + setting_value = getattr(settings, setting_name, None) + + # Only include settings that have non-None and non-empty values + if setting_value is not None and setting_value != "": + # Apply transformer function if available + transformer = self.setting_transformers.get(setting_name) + if transformer: + setting_value = transformer(setting_value) + + # Skip migration if transformer returned None or empty string + if setting_value is not None and setting_value != "": + found_configs.append( + { + 'category': 'global-settings', + 'setting_name': setting_name, + 'setting_value': setting_value, + 'org_mappers': [], # Settings don't have mappers + 'team_mappers': [], # Settings don't have mappers + 'role_mappers': [], # Settings don't have mappers + 'allow_mappers': [], # Settings don't have mappers + } + ) + else: + self._write_output(f'\nIgnoring {setting_name} because it is None or empty after transformation') + else: + self._write_output(f'\nIgnoring {setting_name} because it is None or empty') + + return found_configs + + def create_gateway_authenticator(self, config): + """ + Migrate AWX settings to Gateway. + + Note: This doesn't create authenticators, but updates Gateway settings. + """ + setting_name = config['setting_name'] + setting_value = config['setting_value'] + + self._write_output(f'\n--- Migrating setting: {setting_name} ---') + self._write_output(f'New value: {setting_value}') + + try: + gateway_setting_name = self._convert_setting_name(setting_name) + + # Use the new update_gateway_setting method + self.gateway_client.update_gateway_setting(gateway_setting_name, setting_value) + + self._write_output(f'✓ Successfully migrated setting: {setting_name}', 'success') + + # Return success result in the expected format + return {'success': True, 'action': 'updated', 'error': None} + + except Exception as e: + self._write_output(f'✗ Failed to migrate setting {setting_name}: {str(e)}', 'error') + return {'success': False, 'action': 'failed', 'error': str(e)} + + def migrate(self): + """ + Main entry point - orchestrates the settings migration process. + + Returns: + dict: Summary of migration results + """ + # Get settings from AWX/Controller + configs = self.get_controller_config() + + if not configs: + self._write_output('No settings found to migrate.', 'warning') + return { + 'created': 0, + 'updated': 0, + 'unchanged': 0, + 'failed': 0, + 'mappers_created': 0, + 'mappers_updated': 0, + 'mappers_failed': 0, + 'settings_created': 0, + 'settings_updated': 0, + 'settings_unchanged': 0, + 'settings_failed': 0, + } + + self._write_output(f'Found {len(configs)} setting(s) to migrate.', 'success') + + # Process each setting + created_settings = [] + updated_settings = [] + unchanged_settings = [] + failed_settings = [] + + for config in configs: + result = self.create_gateway_authenticator(config) + if result['success']: + if result['action'] == 'created': + created_settings.append(config) + elif result['action'] == 'updated': + updated_settings.append(config) + elif result['action'] == 'skipped': + unchanged_settings.append(config) + else: + failed_settings.append(config) + + # Settings don't have mappers, or authenticators, so authenticator and mapper counts are always 0 + return { + 'created': 0, + 'updated': 0, + 'unchanged': 0, + 'failed': 0, + 'mappers_created': 0, + 'mappers_updated': 0, + 'mappers_failed': 0, + 'settings_created': len(created_settings), + 'settings_updated': len(updated_settings), + 'settings_unchanged': len(unchanged_settings), + 'settings_failed': len(failed_settings), + }