From b2c0985091797aedb03b5cdccf9d34c6da92a35c Mon Sep 17 00:00:00 2001 From: Quanah Gibson-Mount Date: Thu, 8 Oct 2026 18:26:39 +0000 Subject: [PATCH 1/2] Allow creating a local user without a password validate_password rejected a new user with no password, although _update_password already stores an unusable password in that case. A token-only service account then needs a password nobody should use. Return early for a new user with no password; supplied passwords keep every existing check, and updates are unchanged. related #1089 Assisted-by: Claude Opus 5.5 Signed-off-by: Quanah Gibson-Mount --- ascender/api/serializers/user.py | 7 ++++-- .../main/tests/functional/api/test_user.py | 23 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/ascender/api/serializers/user.py b/ascender/api/serializers/user.py index 9437438c5..6b3e62342 100644 --- a/ascender/api/serializers/user.py +++ b/ascender/api/serializers/user.py @@ -110,9 +110,12 @@ def validate_preferred_theme(self, value): return value def validate_password(self, value): - django_validate_password(value) + # A new user may be created without a password. _update_password then stores an unusable + # one, which suits accounts that only ever authenticate with an OAuth2 token. The checks + # below apply to passwords that are set, not to their absence. if not self.instance and value in (None, ''): - raise serializers.ValidationError(_('Password required for new User.')) + return value + django_validate_password(value) # Check if a password is too long password_max_length = User._meta.get_field('password').max_length diff --git a/ascender/main/tests/functional/api/test_user.py b/ascender/main/tests/functional/api/test_user.py index bc6354c55..b5d5a5d90 100644 --- a/ascender/main/tests/functional/api/test_user.py +++ b/ascender/main/tests/functional/api/test_user.py @@ -4,6 +4,7 @@ import pytest from django.contrib.sessions.middleware import SessionMiddleware +from django.test.utils import override_settings from ascender.main.models import User from ascender.api.versioning import reverse @@ -23,6 +24,28 @@ def test_user_create(post, admin): assert not response.data['is_system_auditor'] +@pytest.mark.django_db +@pytest.mark.parametrize("password", [None, ""]) +def test_user_create_without_password(post, admin, password): + """A user created without a password gets an unusable one, for token-only accounts.""" + user_attrs = {k: v for k, v in EXAMPLE_USER_DATA.items() if k != "password"} + if password is not None: + user_attrs["password"] = password + response = post(reverse('api:user_list'), user_attrs, admin, middleware=SessionMiddleware(mock.Mock())) + assert response.status_code == 201 + assert not User.objects.get(username=user_attrs["username"]).has_usable_password() + + +@pytest.mark.django_db +@override_settings(LOCAL_PASSWORD_MIN_LENGTH=12) +def test_user_create_without_password_ignores_complexity_settings(post, admin): + """The local password rules govern a password that is set; they don't require one.""" + user_attrs = {k: v for k, v in EXAMPLE_USER_DATA.items() if k != "password"} + response = post(reverse('api:user_list'), user_attrs, admin, middleware=SessionMiddleware(mock.Mock())) + assert response.status_code == 201 + assert not User.objects.get(username=user_attrs["username"]).has_usable_password() + + @pytest.mark.django_db def test_fail_double_create_user(post, admin): response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin, middleware=SessionMiddleware(mock.Mock())) From 4a06149b208bdf4712c2cd2794809d45c3365943 Mon Sep 17 00:00:00 2001 From: Quanah Gibson-Mount Date: Mon, 12 Oct 2026 16:47:06 +0000 Subject: [PATCH 2/2] Stop treating every password-less user as a RADIUS user user_is_in_enterprise_category() accepted any user without a usable password as a RADIUS user, a fallback for accounts created before UserEnterpriseAuth existed. With password-less local users now creatable through the API, a RADIUS principal with the same username could log in as such an account, superuser included. Drop the fallback, so RADIUS needs a provider='radius' UserEnterpriseAuth row, as TACACS+ already does. Every RADIUS user created since that model was introduced (2017, before AWX 1.0.0) has one, so existing RADIUS users keep logging in. Tests: a password-less local user is not adopted by RADIUS, through the helper and end to end through RADIUSBackend; a user with a RADIUS row still is. related #1089 Assisted-by: Claude Opus 5.5 Signed-off-by: Quanah Gibson-Mount --- ascender/main/models/__init__.py | 8 +++--- .../main/tests/functional/api/test_user.py | 15 +++++++++++ .../test_get_or_set_enterprise_user.py | 25 +++++++++++++++++++ 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/ascender/main/models/__init__.py b/ascender/main/models/__init__.py index 94233a746..f1d05f401 100644 --- a/ascender/main/models/__init__.py +++ b/ascender/main/models/__init__.py @@ -228,11 +228,9 @@ def user_is_system_auditor(user, tf): def user_is_in_enterprise_category(user, category): ret = (category,) in user.enterprise_auth.values_list('provider') and not user.has_usable_password() - # NOTE: this if-else block ensures existing enterprise users are still able to - # log in. Remove it in a future release - if category == 'radius': - ret = ret or not user.has_usable_password() - elif category == 'saml': + # NOTE: this ensures existing enterprise users are still able to log in. + # Remove it in a future release + if category == 'saml': ret = ret or user.social_auth.all() return ret diff --git a/ascender/main/tests/functional/api/test_user.py b/ascender/main/tests/functional/api/test_user.py index b5d5a5d90..45e23db91 100644 --- a/ascender/main/tests/functional/api/test_user.py +++ b/ascender/main/tests/functional/api/test_user.py @@ -8,6 +8,7 @@ from ascender.main.models import User from ascender.api.versioning import reverse +from ascender.sso.backends import RADIUSBackend # # user creation @@ -46,6 +47,20 @@ def test_user_create_without_password_ignores_complexity_settings(post, admin): assert not User.objects.get(username=user_attrs["username"]).has_usable_password() +@pytest.mark.django_db +@override_settings(RADIUS_SERVER='radius.example.org') +def test_user_created_without_password_is_not_a_radius_user(post, admin): + """A RADIUS principal with the same username must not take over a password-less local account.""" + user_attrs = {k: v for k, v in EXAMPLE_USER_DATA.items() if k != "password"} + user_attrs["is_superuser"] = True + response = post(reverse('api:user_list'), user_attrs, admin, middleware=SessionMiddleware(mock.Mock())) + assert response.status_code == 201 + with mock.patch.object(RADIUSBackend, '_radius_auth', return_value=([], False, False)): + assert RADIUSBackend().authenticate(None, user_attrs["username"], "radius-password") is None + # The stubbed reply does log in a username that has no local account. + assert RADIUSBackend().authenticate(None, "radius-only", "radius-password").username == "radius-only" + + @pytest.mark.django_db def test_fail_double_create_user(post, admin): response = post(reverse('api:user_list'), EXAMPLE_USER_DATA, admin, middleware=SessionMiddleware(mock.Mock())) diff --git a/ascender/sso/tests/functional/test_get_or_set_enterprise_user.py b/ascender/sso/tests/functional/test_get_or_set_enterprise_user.py index dd367842b..5cac7b7b7 100644 --- a/ascender/sso/tests/functional/test_get_or_set_enterprise_user.py +++ b/ascender/sso/tests/functional/test_get_or_set_enterprise_user.py @@ -2,8 +2,12 @@ import pytest from unittest import mock +# Django +from django.contrib.auth.models import User + # Ascender from ascender.sso.backends import _get_or_set_enterprise_user +from ascender.sso.models import UserEnterpriseAuth @pytest.mark.django_db @@ -35,3 +39,24 @@ def test_non_enterprise_user_does_not_get_pass(existing_normal_user): new_user = _get_or_set_enterprise_user("alice", "password", "tacacs+") mocked_logger.warning.assert_called_once_with('Enterprise user alice already defined in Tower.') assert new_user is None + + +@pytest.mark.django_db +def test_radius_does_not_adopt_local_user_without_password(): + """A local user with no usable password, such as a token-only account, is not a RADIUS user.""" + local_user = User.objects.create(username="svc", is_superuser=True) + local_user.set_unusable_password() + local_user.save() + with mock.patch('ascender.sso.backends.logger') as mocked_logger: + assert _get_or_set_enterprise_user("svc", "password", "radius") is None + mocked_logger.warning.assert_called_once_with('Enterprise user svc already defined in Tower.') + assert not UserEnterpriseAuth.objects.filter(user=local_user).exists() + + +@pytest.mark.django_db +def test_radius_user_with_enterprise_auth_logs_in(): + radius_user = User.objects.create(username="bob") + radius_user.set_unusable_password() + radius_user.save() + UserEnterpriseAuth.objects.create(user=radius_user, provider='radius') + assert _get_or_set_enterprise_user("bob", "password", "radius") == radius_user