From 300605ff7333e23823b897fca84ad67c9a6ee884 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Thu, 9 Oct 2025 16:57:58 -0400 Subject: [PATCH] Make subscriptions credentials mutually exclusive (#16126) settings.SUBSCRIPTIONS_USERNAME and settings.SUBSCRIPTIONS_CLIENT_ID should be mutually exclusive. This is because the POST to api/v2/config/attach/ accepts only a subscription_id, and infers which credentials to use based on settings. If both are set, it is ambiguous and can lead to unexpected 400s when attempting to attach a license. Signed-off-by: Seth Foster --- awx/api/views/root.py | 16 ++-- awx/main/conf.py | 4 + .../tests/functional/api/test_licensing.py | 85 ++++++++++--------- 3 files changed, 59 insertions(+), 46 deletions(-) diff --git a/awx/api/views/root.py b/awx/api/views/root.py index 1fa45ad224..9b513337ad 100644 --- a/awx/api/views/root.py +++ b/awx/api/views/root.py @@ -208,18 +208,19 @@ class ApiV2SubscriptionView(APIView): settings.SUBSCRIPTIONS_USERNAME = user if pw: settings.SUBSCRIPTIONS_PASSWORD = pw + # mutual exclusion for basic auth and service account + # only one should be set at a given time so that + # config/attach/ knows which credentials to use + settings.SUBSCRIPTIONS_CLIENT_ID = "" + settings.SUBSCRIPTIONS_CLIENT_SECRET = "" else: if user: settings.SUBSCRIPTIONS_CLIENT_ID = user - if not settings.REDHAT_USERNAME: - # plumb these to analytics credentials - settings.REDHAT_USERNAME = user if pw: settings.SUBSCRIPTIONS_CLIENT_SECRET = pw - if not settings.REDHAT_PASSWORD: - # plumb these to analytics credentials - settings.REDHAT_PASSWORD = pw - + # mutual exclusion for basic auth and service account + settings.SUBSCRIPTIONS_USERNAME = "" + settings.SUBSCRIPTIONS_PASSWORD = "" except Exception as exc: msg = _("Invalid Subscription") if isinstance(exc, TokenError) or ( @@ -283,6 +284,7 @@ class ApiV2AttachView(APIView): else: logger.exception(smart_str(u"Invalid subscription submitted."), extra=dict(actor=request.user.username)) return Response({"error": msg}, status=status.HTTP_400_BAD_REQUEST) + for sub in validated: if sub['subscription_id'] == subscription_id: sub['valid_key'] = True diff --git a/awx/main/conf.py b/awx/main/conf.py index 06852ff3f8..f26e9f53a6 100644 --- a/awx/main/conf.py +++ b/awx/main/conf.py @@ -155,6 +155,7 @@ register( help_text=_('Username used to retrieve subscription and content information'), # noqa category=_('System'), category_slug='system', + hidden=True, ) register( @@ -168,6 +169,7 @@ register( help_text=_('Password used to retrieve subscription and content information'), # noqa category=_('System'), category_slug='system', + hidden=True, ) @@ -182,6 +184,7 @@ register( help_text=_('Client ID used to retrieve subscription and content information'), # noqa category=_('System'), category_slug='system', + hidden=True, ) register( @@ -195,6 +198,7 @@ register( help_text=_('Client secret used to retrieve subscription and content information'), # noqa category=_('System'), category_slug='system', + hidden=True, ) register( diff --git a/awx/main/tests/functional/api/test_licensing.py b/awx/main/tests/functional/api/test_licensing.py index efec73ae3f..a752f0d3f2 100644 --- a/awx/main/tests/functional/api/test_licensing.py +++ b/awx/main/tests/functional/api/test_licensing.py @@ -164,45 +164,6 @@ class TestApiV2SubscriptionView: assert settings.SUBSCRIPTIONS_CLIENT_ID == 'new_client_id' assert settings.SUBSCRIPTIONS_CLIENT_SECRET == 'new_client_secret' - def test_analytics_credentials_plumbed_when_missing(self, post, admin, settings): - """Test that service account credentials are plumbed to analytics when REDHAT_* settings are missing""" - data = {'subscriptions_client_id': 'analytics_client_id', 'subscriptions_client_secret': 'analytics_client_secret'} - - # Ensure REDHAT_* settings are not set - settings.REDHAT_USERNAME = None - settings.REDHAT_PASSWORD = None - - with patch('awx.api.views.root.get_licenser') as mock_get_licenser: - mock_licenser = MagicMock() - mock_licenser.validate_rh.return_value = [] - mock_get_licenser.return_value = mock_licenser - - response = post(reverse('api:api_v2_subscription_view'), data, admin) - - assert response.status_code == status.HTTP_200_OK - assert settings.REDHAT_USERNAME == 'analytics_client_id' - assert settings.REDHAT_PASSWORD == 'analytics_client_secret' - - def test_analytics_credentials_not_overwritten_when_present(self, post, admin, settings): - """Test that existing REDHAT_* settings are not overwritten""" - data = {'subscriptions_client_id': 'new_client_id', 'subscriptions_client_secret': 'new_client_secret'} - - # Set existing REDHAT_* settings - settings.REDHAT_USERNAME = 'existing_username' - settings.REDHAT_PASSWORD = 'existing_password' - - with patch('awx.api.views.root.get_licenser') as mock_get_licenser: - mock_licenser = MagicMock() - mock_licenser.validate_rh.return_value = [] - mock_get_licenser.return_value = mock_licenser - - response = post(reverse('api:api_v2_subscription_view'), data, admin) - - assert response.status_code == status.HTTP_200_OK - # REDHAT_* settings should remain unchanged - assert settings.REDHAT_USERNAME == 'existing_username' - assert settings.REDHAT_PASSWORD == 'existing_password' - def test_validate_rh_exception_handling(self, post, admin): """Test that exceptions from validate_rh are properly handled""" data = {'subscriptions_username': 'test_user', 'subscriptions_password': 'test_password'} @@ -235,3 +196,49 @@ class TestApiV2SubscriptionView: assert response.status_code == status.HTTP_200_OK # Should use service account (basic_auth=False) since client_id is present mock_licenser.validate_rh.assert_called_once_with('test_client_id', 'test_client_secret', False) + + def test_basic_auth_clears_service_account_settings(self, post, admin, settings): + """Test that setting basic auth credentials clears service account settings""" + # Pre-populate service account settings + settings.SUBSCRIPTIONS_CLIENT_ID = 'existing_client_id' + settings.SUBSCRIPTIONS_CLIENT_SECRET = 'existing_client_secret' + + data = {'subscriptions_username': 'test_user', 'subscriptions_password': 'test_password'} + + with patch('awx.api.views.root.get_licenser') as mock_get_licenser: + mock_licenser = MagicMock() + mock_licenser.validate_rh.return_value = [] + mock_get_licenser.return_value = mock_licenser + + response = post(reverse('api:api_v2_subscription_view'), data, admin) + + assert response.status_code == status.HTTP_200_OK + # Basic auth settings should be set + assert settings.SUBSCRIPTIONS_USERNAME == 'test_user' + assert settings.SUBSCRIPTIONS_PASSWORD == 'test_password' + # Service account settings should be cleared + assert settings.SUBSCRIPTIONS_CLIENT_ID == "" + assert settings.SUBSCRIPTIONS_CLIENT_SECRET == "" + + def test_service_account_clears_basic_auth_settings(self, post, admin, settings): + """Test that setting service account credentials clears basic auth settings""" + # Pre-populate basic auth settings + settings.SUBSCRIPTIONS_USERNAME = 'existing_username' + settings.SUBSCRIPTIONS_PASSWORD = 'existing_password' + + data = {'subscriptions_client_id': 'test_client_id', 'subscriptions_client_secret': 'test_client_secret'} + + with patch('awx.api.views.root.get_licenser') as mock_get_licenser: + mock_licenser = MagicMock() + mock_licenser.validate_rh.return_value = [] + mock_get_licenser.return_value = mock_licenser + + response = post(reverse('api:api_v2_subscription_view'), data, admin) + + assert response.status_code == status.HTTP_200_OK + # Service account settings should be set + assert settings.SUBSCRIPTIONS_CLIENT_ID == 'test_client_id' + assert settings.SUBSCRIPTIONS_CLIENT_SECRET == 'test_client_secret' + # Basic auth settings should be cleared + assert settings.SUBSCRIPTIONS_USERNAME == "" + assert settings.SUBSCRIPTIONS_PASSWORD == ""