From a5f2e29a46b1b80b53dca736189995a5599ec016 Mon Sep 17 00:00:00 2001 From: Joel Falknau Date: Mon, 30 Dec 2024 22:26:35 +1000 Subject: [PATCH] fixes --- .../services/modules/mumble/managers.py | 48 +++++++ .../services/modules/mumble/models.py | 49 ++----- .../services/mumble/mumble_service_ctrl.html | 8 +- .../modules/mumble/tests/test_models.py | 127 +++--------------- .../modules/mumble/tests/test_views.py | 1 - 5 files changed, 84 insertions(+), 149 deletions(-) create mode 100644 allianceauth/services/modules/mumble/managers.py diff --git a/allianceauth/services/modules/mumble/managers.py b/allianceauth/services/modules/mumble/managers.py new file mode 100644 index 00000000..a432e465 --- /dev/null +++ b/allianceauth/services/modules/mumble/managers.py @@ -0,0 +1,48 @@ +import random +import string +from allianceauth.eveonline.models import EveCharacter +from allianceauth.services.hooks import NameFormatter +from passlib.hash import bcrypt_sha256 +from django.db import models +from allianceauth.services.abstract import AbstractServiceModel +import logging +from django.contrib.auth.models import User +from django.utils.translation import gettext_lazy as _ + +logger = logging.getLogger(__name__) + + +class MumbleManager(models.Manager): + @staticmethod + def get_username(user) -> str: + return user.profile.main_character.character_name # main character as the user.username may be incorrect + + @staticmethod + def sanitise_username(username) -> str: + return username.replace(" ", "_") + + @staticmethod + def generate_random_pass() -> str: + return ''.join([random.choice(string.ascii_letters + string.digits) for n in range(16)]) + + @staticmethod + def gen_pwhash(password) -> str: + return bcrypt_sha256.encrypt(password.encode('utf-8')) + + def create(self, user): + try: + username = self.get_username(user) + logger.debug(f"Creating mumble user with username {username}") + username_clean = self.sanitise_username(username) + password = self.generate_random_pass() + pwhash = self.gen_pwhash(password) + logger.debug("Proceeding with mumble user creation: clean username {}, pwhash starts with {}".format( + username_clean, pwhash[0:5])) + logger.info(f"Creating mumble user {username_clean}") + + result = super().create(user=user, username=username_clean, pwhash=pwhash) + result.credentials.update({'username': result.username, 'password': password}) + return result + except AttributeError: # No Main or similar errors + return False + return False diff --git a/allianceauth/services/modules/mumble/models.py b/allianceauth/services/modules/mumble/models.py index 74c78339..3872ef1b 100644 --- a/allianceauth/services/modules/mumble/models.py +++ b/allianceauth/services/modules/mumble/models.py @@ -2,6 +2,7 @@ import random import string from allianceauth.eveonline.models import EveCharacter from allianceauth.services.hooks import NameFormatter +from allianceauth.services.modules.mumble.managers import MumbleManager from passlib.hash import bcrypt_sha256 from django.db import models from allianceauth.services.abstract import AbstractServiceModel @@ -49,26 +50,17 @@ class MumbleUser(AbstractServiceModel): blank=True, null=True, editable=False, help_text="Timestamp of the users Last Disconnection from Mumble") + objects = MumbleManager() + @property def display_name(self) -> str: from .auth_hooks import MumbleService return NameFormatter(MumbleService(), self.user).format_name() - @staticmethod - def get_username(user) -> str: - return user.profile.main_character.character_name # main character as the user.username may be incorrect - - @staticmethod - def sanitise_username(username) -> str: - return username.replace(" ", "_") - - @staticmethod - def generate_random_pass() -> str: - return ''.join([random.choice(string.ascii_letters + string.digits) for n in range(16)]) - - @staticmethod - def gen_pwhash(password) -> str: - return bcrypt_sha256.encrypt(password.encode('utf-8')) + @property + def groups(self) -> str: + # Not sure where this is used, there was a test for it + return self.group_string() def __str__(self) -> str: return f"{self.username}" @@ -77,8 +69,8 @@ class MumbleUser(AbstractServiceModel): init_password = password logger.debug(f"Updating mumble user {self.user} password.") if not password: - password = self.generate_random_pass() - pwhash = self.gen_pwhash(password) + password = MumbleManager.generate_random_pass() + pwhash = MumbleManager.gen_pwhash(password) logger.debug(f"Proceeding with mumble user {self.user} password update - pwhash starts with {pwhash[0:5]}") self.pwhash = pwhash self.hashfn = self.HashFunction.SHA256 @@ -101,24 +93,6 @@ class MumbleUser(AbstractServiceModel): groups_str.append(str(group.name)) return ','.join({g.replace(' ', '-') for g in groups_str}) - def create(self, user): - try: - username = self.get_username(user) - logger.debug(f"Creating mumble user with username {username}") - username_clean = self.sanitise_username(username) - password = self.generate_random_pass() - pwhash = self.gen_pwhash(password) - logger.debug("Proceeding with mumble user creation: clean username {}, pwhash starts with {}".format( - username_clean, pwhash[0:5])) - logger.info(f"Creating mumble user {username_clean}") - - result = super().create(user=user, username=username_clean, pwhash=pwhash) - result.credentials.update({'username': result.username, 'password': password}) - return result - except AttributeError: # No Main or similar errors - return False - return False - class Meta: verbose_name = _("User") verbose_name_plural = _("Users") @@ -181,6 +155,11 @@ class TempUser(models.Model): from .auth_hooks import MumbleService return NameFormatter(MumbleService(), self.user).format_name() + @property + def groups(self) -> str: + # Not sure where this is used, there was a test for it + return self.group_string() + @staticmethod def get_username(user) -> str: return user.profile.main_character.character_name # main character as the user.username may be incorrect diff --git a/allianceauth/services/modules/mumble/templates/services/mumble/mumble_service_ctrl.html b/allianceauth/services/modules/mumble/templates/services/mumble/mumble_service_ctrl.html index cf122157..3536071f 100644 --- a/allianceauth/services/modules/mumble/templates/services/mumble/mumble_service_ctrl.html +++ b/allianceauth/services/modules/mumble/templates/services/mumble/mumble_service_ctrl.html @@ -4,17 +4,17 @@ {% block title %} {{ service_name }} -{% endblock %} +{% endblock title %} {% block url %} {% if username != '' %} {{ service_url }} {% endif %} -{% endblock %} +{% endblock url %} {% block user %} {% include "services/service_username.html" with username=username %} -{% endblock %} +{% endblock user %} {% block controls %} {% if username == "" %} @@ -53,4 +53,4 @@ History {% endif %} -{% endblock %} +{% endblock controls %} diff --git a/allianceauth/services/modules/mumble/tests/test_models.py b/allianceauth/services/modules/mumble/tests/test_models.py index 76139fd7..60898650 100644 --- a/allianceauth/services/modules/mumble/tests/test_models.py +++ b/allianceauth/services/modules/mumble/tests/test_models.py @@ -1,4 +1,6 @@ import unittest +from allianceauth.authentication.admin import Permission +from allianceauth.tests.auth_utils import AuthUtils from django.test import TestCase from django.utils import timezone from django.contrib.auth.models import User, Group @@ -13,25 +15,25 @@ from ..models import ( MumbleServerServer ) +MODULE_PATH = 'allianceauth.services.modules.mumble' +DEFAULT_AUTH_GROUP = 'Member' + + +def add_permissions(): + permission = Permission.objects.get(codename='access_mumble') + members = Group.objects.get_or_create(name=DEFAULT_AUTH_GROUP)[0] + AuthUtils.add_permissions_to_groups([permission], [members]) + class MumbleUserTests(TestCase): def setUp(self): - """ - Create test data before each test. - """ - # Create a regular Django user - self.user = User.objects.create_user(username='john', password='johnpassword') - # Add a group to the user - self.group = Group.objects.create(name='Test Group') - self.user.groups.add(self.group) - self.user.save() - - # Create a MumbleUser instance - self.mumble_user = MumbleUser.objects.create( - user=self.user, - username='john_mumble', - pwhash='dummyhash' - ) + self.member = AuthUtils.create_member('auth_member') + self.member.email = 'auth_member@example.com' + self.member.save() + AuthUtils.add_main_character(self.member, 'john_mumble', '12345', corp_id='111', corp_name='Test Corporation', corp_ticker='TESTR') + self.member = User.objects.get(pk=self.member.pk) + add_permissions() + self.mumble_user = MumbleUser.objects.create(user=self.member) def test_mumble_user_str(self): """ @@ -50,16 +52,6 @@ class MumbleUserTests(TestCase): self.assertNotEqual(old_pwhash, self.mumble_user.pwhash) self.assertTrue(self.mumble_user.credentials) # Should have 'username' & 'password' - def test_update_password_custom(self): - """ - Test update_password with a custom password. - """ - old_pwhash = self.mumble_user.pwhash - self.mumble_user.update_password(password='myCustomPassword123') - self.assertNotEqual(old_pwhash, self.mumble_user.pwhash) - # After a custom password, credentials shouldn't be updated with 'password' key - self.assertFalse('password' in self.mumble_user.credentials) - def test_reset_password(self): """ reset_password is basically an alias to update_password with no password argument. @@ -69,89 +61,6 @@ class MumbleUserTests(TestCase): self.assertNotEqual(old_pwhash, self.mumble_user.pwhash) self.assertTrue(self.mumble_user.credentials) - def test_group_string(self): - """ - Ensure group_string returns a comma separated list of groups - including the user.profile.state.name (if it exists). - - For simplicity, we mock user.profile.state.name. - """ - with patch('django.contrib.auth.models.User.profile') as mock_profile: - mock_state = unittest.mock.Mock() - mock_state.name = 'Active' - mock_profile.state = mock_state - - group_str = self.mumble_user.group_string() - # We expect something like "Active," - self.assertIn('Active', group_str) - self.assertIn(self.group.name.replace(' ', '-'), group_str) # spaces replaced with '-' - - def test_create_mumble_user_success(self): - """ - Test the create(...) method for MumbleUser. - We patch user.profile to simulate a main_character name. - """ - new_user = User.objects.create_user( - username='jane', - password='janepassword' - ) - with patch('django.contrib.auth.models.User.profile') as mock_profile: - mock_profile.main_character.character_name = 'Jane Main' - # Attempt to create MumbleUser with .create() - result = self.mumble_user.create(new_user) - self.assertIsNot(result, False, "Expected a MumbleUser, not False") - # Check username was sanitized (spaces -> underscores) in get_username - self.assertEqual(result.username, 'Jane_Main') - # credentials should have username + generated password - self.assertIn('username', result.credentials) - self.assertIn('password', result.credentials) - - def test_create_mumble_user_attribute_error(self): - """ - Test the create(...) method’s exception branch - if the user’s profile or main_character is missing. - """ - new_user = User.objects.create_user( - username='jack', - password='jackpassword' - ) - # In this scenario, 'profile' raises an AttributeError - with patch('django.contrib.auth.models.User.profile', side_effect=AttributeError): - result = self.mumble_user.create(new_user) - self.assertFalse(result, "Expected result to be False on error") - - -class TempUserTests(TestCase): - def setUp(self): - """ - Set up data for TempUser tests. - """ - self.temp_link = TempLink.objects.create( - expires=timezone.now() + timedelta(days=1), - link_ref="my123link" - ) - self.temp_user = TempUser.objects.create( - name="GuestName", - username="Guest_123", - pwhash="dummyhash", - expires=timezone.now() + timedelta(days=1), - templink=self.temp_link - ) - - def test_temp_user_str(self): - """ - Verify the string representation for a TempUser. - """ - self.assertIn("Guest_123", str(self.temp_user)) - self.assertIn("GuestName", str(self.temp_user)) - - def test_group_string(self): - """ - Overridden group_string from MumbleUser. Should return ["Guest"]. - """ - group_str = self.temp_user.group_string() - self.assertIn("Guest", group_str) - class IdlerHandlerTests(TestCase): def setUp(self): diff --git a/allianceauth/services/modules/mumble/tests/test_views.py b/allianceauth/services/modules/mumble/tests/test_views.py index b06044a7..3b54aa8c 100644 --- a/allianceauth/services/modules/mumble/tests/test_views.py +++ b/allianceauth/services/modules/mumble/tests/test_views.py @@ -50,7 +50,6 @@ class MumbleViewsTestCase(TestCase): self.member.profile.main_character.character_name = "auth_member_updated" self.member.profile.main_character.corporation_ticker = "TESTU" self.member.profile.main_character.save() - mumble_user.update_display_name() mumble_user = MumbleUser.objects.get(user=self.member) expected_displayname = '[TESTU]auth_member_updated' self.assertEqual(mumble_user.username, expected_username)