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/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 bc6354c55..45e23db91 100644 --- a/ascender/main/tests/functional/api/test_user.py +++ b/ascender/main/tests/functional/api/test_user.py @@ -4,9 +4,11 @@ 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 +from ascender.sso.backends import RADIUSBackend # # user creation @@ -23,6 +25,42 @@ 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 +@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