From ee6b8d3e100e1417dfd509598e7753851d475d7b Mon Sep 17 00:00:00 2001 From: catborise Date: Fri, 6 Nov 2026 11:17:59 +0300 Subject: [PATCH] test: no seeded admin password, settings changed only for a test, no local libvirt by default - create_admin had a branch for `"test" in sys.argv` that gave the admin the password "admin"; 17 places signed in with it, and failed silently with ADMIN_PASSWORD set in the environment. The tests now make their own superuser and force_login it, and the shipped code has no test branch: the test runner gives migrate a password (also when ADMIN_PASSWORD is set but empty), so no data/admin_password is written into the checkout. - AccountsTestCase builds its rows in setUpTestData instead of setUpClass and deleting them by hand. - accounts tests set OTP_ENABLED with override_settings instead of assigning settings (one assignment was never undone, so later tests in the process ran with OTP on), and undo their app_settings change. The OTP login class reloads the URLs again only after the setting is off. - computes/tests/test_views.py reached the machine's own libvirt (localhost over the socket) when TEST_LIBVIRT_HOST was unset; its live tests now run only with TEST_LIBVIRT_HOST, like the other live tests, as the README says. --- README.md | 2 +- accounts/apps.py | 13 +++----- accounts/tests/test_admin_password.py | 2 +- accounts/tests/test_otp_login.py | 44 +++++++++++---------------- accounts/tests/test_views.py | 24 +++++---------- admin/tests/test_views.py | 8 +++-- appsettings/tests/test_appsettings.py | 6 ++-- computes/tests/test_views.py | 8 +++-- datasource/tests/test_datasource.py | 3 +- instances/tests/test_root_pass.py | 17 ++++++----- instances/tests/test_views.py | 5 +-- webvirtcloud/test_runner.py | 9 ++++-- 12 files changed, 67 insertions(+), 74 deletions(-) diff --git a/README.md b/README.md index 1cee0540..e8accf7c 100644 --- a/README.md +++ b/README.md @@ -894,7 +894,7 @@ ruff check . Each app keeps its tests in a `tests/` package, so `python manage.py test instances` runs one app; `--parallel` spreads the tests over the CPUs. The suite includes mocked regression tests and disk/clone tests on libvirt's in-process `test:///default` driver. Set `WEBVIRTCLOUD_DB_HOST` (and the other `WEBVIRTCLOUD_DB_*` variables) to run it against PostgreSQL, as CI does. -The compute tests try a local libvirt and skip when none answers; the instance tests run only when `TEST_LIBVIRT_HOST` is set. A green result with skips does not establish live KVM compatibility. The instance tests create `wvc-test-*` VMs and a temporary `wvc-test` pool (`/var/lib/libvirt/wvc-test`), remove them afterwards, and fail if any other VM or volume on the host changed: +The compute and instance tests run only when `TEST_LIBVIRT_HOST` is set, never on the machine's own libvirt by default. A green result with skips does not establish live KVM compatibility. The instance tests create `wvc-test-*` VMs and a temporary `wvc-test` pool (`/var/lib/libvirt/wvc-test`), remove them afterwards, and fail if any other VM or volume on the host changed: ```bash export TEST_LIBVIRT_HOST=compute1 TEST_LIBVIRT_TYPE=2 # 1 TCP, 2 SSH, 3 TLS, 4 socket diff --git a/accounts/apps.py b/accounts/apps.py index 931d6620..56925b7a 100644 --- a/accounts/apps.py +++ b/accounts/apps.py @@ -66,7 +66,6 @@ def create_admin(sender, **kwargs): """ import os import secrets - import sys from django.contrib.auth.models import User from accounts.models import UserAttributes @@ -82,17 +81,13 @@ def create_admin(sender, **kwargs): if User.objects.exists(): return - is_testing = "test" in sys.argv admin_user = os.environ.get("ADMIN_USERNAME", "admin") admin_pass = os.environ.get("ADMIN_PASSWORD") - generated_password = not admin_pass and not is_testing + generated_password = not admin_pass - if not admin_pass: - if is_testing: - admin_pass = "admin" - else: - admin_pass = secrets.token_urlsafe(16) - _store_generated_password(admin_pass) + if generated_password: + admin_pass = secrets.token_urlsafe(16) + _store_generated_password(admin_pass) print(f"\033[1m* \033[92mCreating default admin user '{admin_user}'\033[0m") with transaction.atomic(): diff --git a/accounts/tests/test_admin_password.py b/accounts/tests/test_admin_password.py index 23cb65d4..872920b8 100644 --- a/accounts/tests/test_admin_password.py +++ b/accounts/tests/test_admin_password.py @@ -24,7 +24,7 @@ class GeneratedAdminPasswordTestCase(TestCase): plan = [(migration, False)] out = io.StringIO() env = {k: v for k, v in os.environ.items() if k not in ("ADMIN_PASSWORD", "ADMIN_USERNAME")} - with override_settings(BASE_DIR=Path(base_dir)), patch("sys.argv", ["manage.py", "migrate"]), patch.dict( + with override_settings(BASE_DIR=Path(base_dir)), patch.dict( os.environ, env, clear=True ), redirect_stdout(out): create_admin(sender=None, plan=plan, apps=apps) diff --git a/accounts/tests/test_otp_login.py b/accounts/tests/test_otp_login.py index 10e397b8..878a818f 100644 --- a/accounts/tests/test_otp_login.py +++ b/accounts/tests/test_otp_login.py @@ -3,11 +3,16 @@ enter a valid code; a user without one logs in with the password alone. The device is chosen by the server (a posted device id is ignored), so the single token field of the login page works and no one else's device can be used.""" +import importlib + from django.contrib.auth import get_user_model -from django.test import RequestFactory, TestCase +from django.test import RequestFactory, TestCase, override_settings +from django.urls import clear_url_caches from django_otp.oath import totp from django_otp.plugins.otp_totp.models import TOTPDevice +import accounts.urls +import webvirtcloud.urls from accounts.otp import SingleDeviceOTPForm @@ -84,6 +89,13 @@ class SingleDeviceOTPFormTestCase(TestCase): self.assertFalse(self._form(otp_token=code).is_valid()) +def reload_urls(): + importlib.reload(accounts.urls) + importlib.reload(webvirtcloud.urls) + clear_url_caches() + + +@override_settings(OTP_ENABLED=True) class OTPLoginHTTPTestCase(TestCase): """The actual login page (OTP_ENABLED): the single token field logs an enrolled user in, and a user without a device needs only the password. @@ -92,32 +104,10 @@ class OTPLoginHTTPTestCase(TestCase): @classmethod def setUpClass(cls): - super().setUpClass() - import importlib - from django.conf import settings - from django.urls import clear_url_caches - import accounts.urls - import webvirtcloud.urls - - cls._otp_was_enabled = settings.OTP_ENABLED - settings.OTP_ENABLED = True - importlib.reload(accounts.urls) - importlib.reload(webvirtcloud.urls) - clear_url_caches() - - @classmethod - def tearDownClass(cls): - import importlib - from django.conf import settings - from django.urls import clear_url_caches - import accounts.urls - import webvirtcloud.urls - - settings.OTP_ENABLED = cls._otp_was_enabled - importlib.reload(accounts.urls) - importlib.reload(webvirtcloud.urls) - clear_url_caches() - super().tearDownClass() + # class cleanups run last first: the URLs reload after the setting is off again + cls.addClassCleanup(reload_urls) + super().setUpClass() # turns OTP_ENABLED on + reload_urls() def setUp(self): self.user = get_user_model().objects.create_user("bob", password="pw-correct-horse") diff --git a/accounts/tests/test_views.py b/accounts/tests/test_views.py index ea78cf4c..29d7b0d7 100644 --- a/accounts/tests/test_views.py +++ b/accounts/tests/test_views.py @@ -5,7 +5,7 @@ from django.conf import settings from django.contrib.auth import get_user_model from django.contrib.auth.models import Permission from django.shortcuts import reverse -from django.test import Client, RequestFactory, TestCase +from django.test import Client, RequestFactory, TestCase, override_settings from instances.models import Instance @@ -16,12 +16,9 @@ from accounts.utils import validate_ssh_key class AccountsTestCase(TestCase): @classmethod - def setUpClass(cls): - super().setUpClass() - - # Add users for testing purposes + def setUpTestData(cls): User = get_user_model() - cls.admin_user = User.objects.get(pk=1) + cls.admin_user = User.objects.create_superuser("accounts-admin", "admin@example.com", "pw") cls.test_user = User.objects.create_user(username="test", password="test") # Add compute @@ -38,16 +35,8 @@ class AccountsTestCase(TestCase): name="test-vm", ) - @classmethod - def tearDownClass(cls): - if getattr(cls, "instance", None): - cls.instance.delete() - if getattr(cls, "compute", None): - cls.compute.delete() - super().tearDownClass() - def setUp(self): - self.client.login(request=RequestFactory().get("/"), username="admin", password="admin") + self.client.force_login(self.admin_user) permission = Permission.objects.get(codename="change_password") self.test_user.user_permissions.add(permission) self.rsa_key = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQC6OOdbfv27QVnSC6sKxGaHb6YFc+3gxCkyVR3cTSXE/n5BEGf8aOgBpepULWa1RZfxYHY14PlKULDygdXSdrrR2kNSwoKz/Oo4d+3EE92L7ocl1+djZbptzgWgtw1OseLwbFik+iKlIdqPsH+IUQvX7yV545ZQtAP8Qj1R+uCqkw== test@test" @@ -62,8 +51,8 @@ class AccountsTestCase(TestCase): ) self.assertEqual(response.status_code, 200) + @override_settings(OTP_ENABLED=True) def test_account_with_otp(self): - settings.OTP_ENABLED = True response = self.client.get( reverse("accounts:account", args=[self.test_user.id]) ) @@ -249,7 +238,7 @@ class AccountsTestCase(TestCase): self.assertRedirects(response, reverse("accounts:profile") + "#sshkeys") def test_the_profile_lists_keys_and_tokens_where_links_land(self): - admin = get_user_model().objects.get(username="admin") + admin = self.admin_user key = UserSSHKey.objects.create(user=admin, keyname="laptop", keypublic=self.rsa_key) response = self.client.get(reverse("accounts:profile")) # other pages link to #sshkeys and #apitokens @@ -288,6 +277,7 @@ class AccountsTestCase(TestCase): ) self.assertFalse(form.is_valid()) + self.addCleanup(setattr, app_settings, "ALLOW_INSTANCE_MULTIPLE_OWNER", app_settings.ALLOW_INSTANCE_MULTIPLE_OWNER) app_settings.ALLOW_INSTANCE_MULTIPLE_OWNER = "False" form = UserInstanceForm( { diff --git a/admin/tests/test_views.py b/admin/tests/test_views.py index cb87f869..9f306788 100644 --- a/admin/tests/test_views.py +++ b/admin/tests/test_views.py @@ -1,8 +1,9 @@ +from django.contrib.auth import get_user_model from django.contrib.auth.models import Group, Permission, User from django.core.exceptions import ObjectDoesNotExist from django.db import connection from django.shortcuts import reverse -from django.test import RequestFactory, TestCase +from django.test import TestCase from django.test.utils import CaptureQueriesContext from lxml import html @@ -12,7 +13,8 @@ from logs.models import Logs class AdminTestCase(TestCase): def setUp(self): - self.client.login(request=RequestFactory().get("/"), username="admin", password="admin") + self.admin = get_user_model().objects.create_superuser("tests-admin", "admin@example.com", "pw") + self.client.force_login(self.admin) Logs.objects.all().delete() # the login's row def test_group_list(self): @@ -63,7 +65,7 @@ class AdminTestCase(TestCase): page = html.fromstring(self.client.get(reverse("admin:group_update", args=[group.id])).content) boxes = page.xpath("//div[contains(@class, 'searchable')]/label") names = [box.text_content().split()[0] for box in boxes] - self.assertEqual(names, ["zed", "admin", "amy", "bob"]) + self.assertEqual(names, ["zed", "admin", "amy", "bob", "tests-admin"]) self.assertIn("Amy Adams", boxes[2].get("data-search")) self.assertEqual(page.xpath("//input[@id='filter']/@data-forget"), [""]) self.assertEqual(page.xpath("//form[@id='create-update']//button[@type='submit']/text()"), ["Save changes"]) diff --git a/appsettings/tests/test_appsettings.py b/appsettings/tests/test_appsettings.py index 1114dead..ba277032 100644 --- a/appsettings/tests/test_appsettings.py +++ b/appsettings/tests/test_appsettings.py @@ -1,7 +1,8 @@ import re +from django.contrib.auth import get_user_model from django.contrib.messages import get_messages -from django.test import RequestFactory, TestCase +from django.test import TestCase from django.urls import reverse from appsettings.models import AppSettings @@ -10,7 +11,8 @@ from appsettings.settings import app_settings, get_settings class AppSettingsTestCase(TestCase): def setUp(self): - self.client.login(request=RequestFactory().get("/"), username="admin", password="admin") + self.admin = get_user_model().objects.create_superuser("tests-admin", "admin@example.com", "pw") + self.client.force_login(self.admin) def test_get_settings_populates_singleton(self): AppSettings.objects.update_or_create( diff --git a/computes/tests/test_views.py b/computes/tests/test_views.py index 8b1b4ebc..8022e7ac 100644 --- a/computes/tests/test_views.py +++ b/computes/tests/test_views.py @@ -1,9 +1,10 @@ import json import os from unittest.mock import patch +from django.contrib.auth import get_user_model from django.core.exceptions import ObjectDoesNotExist from django.shortcuts import reverse -from django.test import RequestFactory, TestCase, TransactionTestCase +from django.test import TestCase, TransactionTestCase from vrtManager.connection import CONN_SOCKET, connection_manager from computes.models import Compute @@ -17,7 +18,8 @@ TEST_COMPUTE_TYPE = int(os.environ.get("TEST_LIBVIRT_TYPE", CONN_SOCKET)) class ComputesTestCase(TestCase): def setUp(self): - self.client.login(request=RequestFactory().get("/"), username="admin", password="admin") + self.admin = get_user_model().objects.create_superuser("tests-admin", "admin@example.com", "pw") + self.client.force_login(self.admin) self.compute, _ = Compute.objects.get_or_create( name="computes-test-compute", defaults={ @@ -34,6 +36,8 @@ class ComputesTestCase(TestCase): super().tearDown() def _require_live_compute(self): + if not livetest.enabled(): # never the machine's own libvirt by default + self.skipTest("Set TEST_LIBVIRT_HOST to run the live libvirt tests") try: conn = connection_manager.get_connection( self.compute.hostname, diff --git a/datasource/tests/test_datasource.py b/datasource/tests/test_datasource.py index ae3e7ff8..2f82bcc2 100644 --- a/datasource/tests/test_datasource.py +++ b/datasource/tests/test_datasource.py @@ -16,7 +16,8 @@ from datasource.views import get_client_ip, get_hostname_by_ip class DataSourceTestCase(TestCase): def setUp(self): self.factory = RequestFactory() - self.client.login(request=RequestFactory().get("/"), username="admin", password="admin") + self.admin = get_user_model().objects.create_superuser("tests-admin", "admin@example.com", "pw") + self.client.force_login(self.admin) self.compute = Compute.objects.create( name="test_compute", hostname="localhost", diff --git a/instances/tests/test_root_pass.py b/instances/tests/test_root_pass.py index 49e4dc63..2bcb15bc 100644 --- a/instances/tests/test_root_pass.py +++ b/instances/tests/test_root_pass.py @@ -1,7 +1,8 @@ from unittest.mock import patch +from django.contrib.auth import get_user_model from django.contrib.messages import get_messages -from django.test import RequestFactory, TestCase +from django.test import TestCase from django.urls import reverse from computes.models import Compute @@ -12,7 +13,8 @@ from vrtManager.util import OperationError class SetRootPassTestCase(TestCase): def setUp(self): - self.client.login(request=RequestFactory().get("/"), username="admin", password="admin") + self.admin = get_user_model().objects.create_superuser("tests-admin", "admin@example.com", "pw") + self.client.force_login(self.admin) self.compute = Compute.objects.create( name="test_compute_root_pass", hostname="localhost", @@ -73,9 +75,8 @@ class SetRootPassTestCase(TestCase): @patch.object(Instance, "proxy") def test_unreachable_gstfsd_is_an_error_message_not_a_crash(self, mock_proxy, gstfsd): from accounts.models import UserSSHKey - from django.contrib.auth import get_user_model - admin = get_user_model().objects.get(username="admin") + admin = self.admin key = UserSSHKey.objects.create(user=admin, keyname="k", keypublic="ssh-ed25519 AAAA k") mock_proxy.instance.name.return_value = "root-pass-vm" mock_proxy.get_status.return_value = 5 @@ -94,11 +95,12 @@ class GuestAgentAccessTestCase(TestCase): from accounts.models import UserSSHKey from django.contrib.auth import get_user_model - self.client.login(request=RequestFactory().get("/"), username="admin", password="admin") + self.admin = get_user_model().objects.create_superuser("tests-admin", "admin@example.com", "pw") + self.client.force_login(self.admin) compute = Compute.objects.create(name="agent-compute", hostname="192.0.2.1", type=1) self.instance = Instance.objects.create(compute=compute, name="agent-vm", uuid="11111111-2222-3333-4444-555555555555") - admin = get_user_model().objects.get(username="admin") + admin = self.admin self.key = UserSSHKey.objects.create(user=admin, keyname="laptop", keypublic="ssh-ed25519 AAAA laptop") patcher = patch.object(Instance, "proxy") self.proxy = patcher.start() @@ -175,7 +177,8 @@ class OsInfoErrorTestCase(TestCase): answer in time is an error answer, not a server error.""" def setUp(self): - self.client.login(request=RequestFactory().get("/"), username="admin", password="admin") + self.admin = get_user_model().objects.create_superuser("tests-admin", "admin@example.com", "pw") + self.client.force_login(self.admin) compute = Compute.objects.create(name="osinfo-compute", hostname="192.0.2.1", type=1) self.instance = Instance.objects.create(compute=compute, name="osinfo-vm", uuid="22222222-3333-4444-5555-666666666666") diff --git a/instances/tests/test_views.py b/instances/tests/test_views.py index b6ee2531..fde523d1 100644 --- a/instances/tests/test_views.py +++ b/instances/tests/test_views.py @@ -9,7 +9,7 @@ from django.contrib.auth import get_user_model from django.contrib.auth.models import Permission from django.http.response import Http404 from django.shortcuts import reverse -from django.test import RequestFactory, TestCase +from django.test import TestCase from vrtManager.create import wvmCreate from vrtManager.util import randomUUID @@ -124,7 +124,8 @@ class InstancesTestCase(TestCase): ) def setUp(self): - self.client.login(request=RequestFactory().get("/"), username="admin", password="admin") + self.admin = get_user_model().objects.create_superuser("tests-admin", "admin@example.com", "pw") + self.client.force_login(self.admin) self.rsa_key = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQC6OOdbfv27QVnSC6sKxGaHb6YFc+3gxCkyVR3cTSXE/n5BEGf8aOgBpepULWa1RZfxYHY14PlKULDygdXSdrrR2kNSwoKz/Oo4d+3EE92L7ocl1+djZbptzgWgtw1OseLwbFik+iKlIdqPsH+IUQvX7yV545ZQtAP8Qj1R+uCqkw== test@test" def test_index(self): diff --git a/webvirtcloud/test_runner.py b/webvirtcloud/test_runner.py index 89b0c38b..ab28168f 100644 --- a/webvirtcloud/test_runner.py +++ b/webvirtcloud/test_runner.py @@ -2,6 +2,7 @@ import contextlib import os +import secrets from django.db import connections from django.test.runner import DiscoverRunner @@ -28,9 +29,13 @@ class TestRunner(DiscoverRunner): return self.test_suite([suite, *serial]) def setup_databases(self, **kwargs): + # migrate creates the admin user (accounts.apps.create_admin): with a + # password given it writes no data/admin_password into the checkout + if not os.environ.get("ADMIN_PASSWORD"): # an empty one counts as none + os.environ["ADMIN_PASSWORD"] = secrets.token_urlsafe(16) # Django clones a SQLite test database by copying its main file, but - # migrate's writes are still in the WAL file (operations/apps.py): - # create the database alone, write the WAL back, then clone. + # migrate's writes are still in the WAL file (the init_command's + # journal mode): create the database alone, write the WAL back, then clone. parallel, self.parallel = self.parallel, 0 test_files = [c.settings_dict["TEST"]["NAME"] for c in connections.all() if c.vendor == "sqlite"] for name in filter(None, test_files) if not self.keepdb else ():